diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 444b00d73f0b72162de190c6a6781e145d471aa3..4dca03746ed1639495db666b4370ba10c82c297e 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -119,6 +119,8 @@ def FuchsiaAlpha : Package<"fuchsia">, ParentPackage; def WebKit : Package<"webkit">; def WebKitAlpha : Package<"webkit">, ParentPackage; +def OpenHarmony : Package<"openharmony">; + //===----------------------------------------------------------------------===// // Core Checkers. //===----------------------------------------------------------------------===// @@ -1673,3 +1675,18 @@ def UncountedLocalVarsChecker : Checker<"UncountedLocalVarsChecker">, Documentation; } // end alpha.webkit + +//===----------------------------------------------------------------------===// +// OpenHarmony checkers. +//===----------------------------------------------------------------------===// + +let ParentPackage = OpenHarmony in { +def UnixAPIArgsChecker : Checker<"UnixAPIArgs">, + HelpText<"Check for open unix api arguments">, + Documentation; +} + +def MemcpyChecker : Checker<"Memcpy">, + HelpText<"Check for memcpy_s api arguments">, + Documentation; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index eb4f3013773256fcfa9917a62116ebaf6600b6b5..2a63b9446dcca66f9db782bbea4d87c29051040f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -129,6 +129,8 @@ add_clang_library(clangStaticAnalyzerCheckers WebKit/UncountedCallArgsChecker.cpp WebKit/UncountedLambdaCapturesChecker.cpp WebKit/UncountedLocalVarsChecker.cpp + OpenHarmony/UnixAPIArgsChecker.cpp + OpenHarmony/MemcpyChecker.cpp LINK_LIBS clangAST diff --git a/clang/lib/StaticAnalyzer/Checkers/OpenHarmony/MemcpyChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/OpenHarmony/MemcpyChecker.cpp new file mode 100644 index 0000000000000000000000000000000000000000..fab9e0e5beabdd8b950d7591974fe254877ec2d8 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/OpenHarmony/MemcpyChecker.cpp @@ -0,0 +1,109 @@ +//== MemcpyChecker.cpp ------------------------------*- C++ -*--==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines MemcpyChecker, which is a path-sensitive check +// looking for mismatch src and dest buffer length may cause buffer overflow. +// +//===----------------------------------------------------------------------===// + +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h" + +using namespace clang; +using namespace ento; + +namespace { +class MemcpyChecker : public Checker { + CallDescription MemcpyS; + + std::unique_ptr OverflowBugType; +public: + MemcpyChecker(); + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; +}; + +MemcpyChecker::MemcpyChecker() + : MemcpyS("memcpy_s") { + OverflowBugType.reset( + new BugType(this, "MyFistChecker memcpy_s overflow", "MyFistChecker error")); + } + +void MemcpyChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { + if (!Call.isCalled(MemcpyS)) { + return; + } + + SValBuilder &SVB = C.getSValBuilder(); + ProgramStateRef state = C.getState(); + SVal dstAddrSVal = Call.getArgSVal(0); + SVal srcLengthSVal = Call.getArgSVal(3); + + const MemRegion *dstAddrMR = dstAddrSVal.getAsRegion(); + if (!dstAddrMR) { + return; + } + + const ElementRegion *dstAddrER = dyn_cast(dstAddrMR); + if (!dstAddrER) { + return; + } + + DefinedOrUnknownSVal Idx = dstAddrER->getIndex().castAs(); + Optional IdxSVal = Idx.getAs(); + if (!IdxSVal) { + return; + } + + DefinedOrUnknownSVal ElementCount = getDynamicElementCount( + state, dstAddrER->getSuperRegion(), C.getSValBuilder(), dstAddrER->getValueType()); + + Optional dstAddrLenSVal = ElementCount.getAs(); + if (!dstAddrLenSVal) { + return; + } + + Optional srcLengthDSVal = srcLengthSVal.getAs(); + + SVal srcLenDSval = SVB.evalBinOp(state, BO_Add, *srcLengthDSVal, *IdxSVal, SVB.getArrayIndexType()); + + SVal dstLessThanSrcLength = SVB.evalBinOp(state, BO_LT, *dstAddrLenSVal, srcLenDSval, SVB.getConditionType()); + + Optional dstLessThanSrcLengthDVal = dstLessThanSrcLength.getAs(); + if (!dstLessThanSrcLengthDVal) { + return; + } + + if (state->assume(*dstLessThanSrcLengthDVal, true)) { + // it is possible that dst less than src length + ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); + // If we've already reached this node on another path, return. + if (!ErrNode) + return; + + // Generate the report. + auto R = std::make_unique( + *OverflowBugType, "memcpy_s(): src length may be larger than dst length", ErrNode); + R->addRange(Call.getSourceRange()); + C.emitReport(std::move(R)); + return; + } +} +} + +void ento::registerMemcpyChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} + +// This checker should be enabled regardless of how language options are set. +bool ento::shouldRegisterMemcpyChecker(const CheckerManager &mgr) { + return true; +} diff --git a/clang/lib/StaticAnalyzer/Checkers/OpenHarmony/UnixAPIArgsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/OpenHarmony/UnixAPIArgsChecker.cpp new file mode 100644 index 0000000000000000000000000000000000000000..f704d210c0dbc49f48a8b6e3f6b4de4a144f835e --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/OpenHarmony/UnixAPIArgsChecker.cpp @@ -0,0 +1,249 @@ +//== UnixAPIArgsChecker.cpp ------------------------------*- C++ -*--==// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This defines UnixAPIArgsChecker, which is a path-sensitive checker +// looking for open a file with open() with mode larger than 0644 +// +//===----------------------------------------------------------------------===// + +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/Basic/TargetInfo.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/Support/raw_ostream.h" + +using namespace clang; +using namespace ento; + +enum class OpenVariant { + /// The standard open() call: + /// int open(const char *pathname, int flags, mode_t mode); + Open, + + /// The variant taking a directory file descriptor and a relative path: + /// int openat(int fd, const char *pathname, int flags, mode_t mode); + OpenAt +}; + +namespace { + +class UnixAPIArgsChecker : public Checker< check::PreStmt > { + mutable std::unique_ptr BT_open; + // value of O_CREAT flag + mutable Optional Val_O_CREAT; + // value of mode being checked + mutable Optional Val_MODE; + +public: + void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; + + void CheckOpen(CheckerContext &C, const CallExpr *CE) const; + void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const; + + void CheckOpenVariant(CheckerContext &C, + const CallExpr *CE, OpenVariant Variant) const; + + void ReportOpenBug(CheckerContext &C, + ProgramStateRef State, + const char *Msg, + SourceRange SR) const; + +}; + +} //end anonymous namespace + +static void LazyInitialize(const CheckerBase *Checker, + std::unique_ptr &BT, + const char *name) { + if (BT) + return; + BT.reset(new BugType(Checker, name, categories::UnixAPI)); +} + +//===----------------------------------------------------------------------===// +// "open" (man 2 open) +//===----------------------------------------------------------------------===/ + +void UnixAPIArgsChecker::checkPreStmt(const CallExpr *CE, + CheckerContext &C) const { + const FunctionDecl *FD = C.getCalleeDecl(CE); + if (!FD || FD->getKind() != Decl::Function) + return; + + // Don't treat functions in namespaces with the same name a Unix function + // as a call to the Unix function. + const DeclContext *NamespaceCtx = FD->getEnclosingNamespaceContext(); + if (NamespaceCtx && isa(NamespaceCtx)) + return; + + StringRef FName = C.getCalleeName(FD); + if (FName.empty()) + return; + + if (FName == "open") + CheckOpen(C, CE); + + else if (FName == "openat") + CheckOpenAt(C, CE); +} +void UnixAPIArgsChecker::ReportOpenBug(CheckerContext &C, + ProgramStateRef State, + const char *Msg, + SourceRange SR) const { + ExplodedNode *N = C.generateErrorNode(State); + if (!N) + return; + + LazyInitialize(this, BT_open, "Improper use of 'open'"); + + auto Report = std::make_unique(*BT_open, Msg, N); + Report->addRange(SR); + C.emitReport(std::move(Report)); +} + +void UnixAPIArgsChecker::CheckOpen(CheckerContext &C, + const CallExpr *CE) const { + CheckOpenVariant(C, CE, OpenVariant::Open); +} + +void UnixAPIArgsChecker::CheckOpenAt(CheckerContext &C, + const CallExpr *CE) const { + CheckOpenVariant(C, CE, OpenVariant::OpenAt); +} + +void UnixAPIArgsChecker::CheckOpenVariant(CheckerContext &C, + const CallExpr *CE, + OpenVariant Variant) const { + // The index of the argument taking the flags open flags (O_RDONLY, + // O_WRONLY, O_CREAT, etc.), + unsigned int FlagsArgIndex; + const char *VariantName; + switch (Variant) { + case OpenVariant::Open: + FlagsArgIndex = 1; + VariantName = "open"; + break; + case OpenVariant::OpenAt: + FlagsArgIndex = 2; + VariantName = "openat"; + break; + }; + + // All calls should at least provide arguments up to the 'flags' parameter. + unsigned int MinArgCount = FlagsArgIndex + 1; + + // If the flags has O_CREAT set then open/openat() require an additional + // argument specifying the file mode (permission bits) for the created file. + unsigned int CreateModeArgIndex = FlagsArgIndex + 1; + + // The create mode argument should be the last argument. + unsigned int MaxArgCount = CreateModeArgIndex + 1; + + ProgramStateRef state = C.getState(); + + // Checked via UnixAPIChecker + if (CE->getNumArgs() < MinArgCount || CE->getNumArgs() > MaxArgCount) { + return; + } else if (CE->getNumArgs() == MaxArgCount) { + const Expr *Arg = CE->getArg(CreateModeArgIndex); + QualType QT = Arg->getType(); + if (!QT->isIntegerType()) { + return; + } + } + + if (!Val_O_CREAT.hasValue()) { + Val_O_CREAT = 0100; + } + + // Now check if oflags has O_CREAT set. + const Expr *oflagsEx = CE->getArg(FlagsArgIndex); + const SVal V = C.getSVal(oflagsEx); + if (!V.getAs()) { + // The case where 'V' can be a location can only be due to a bad header, + // so in this case bail out. + return; + } + NonLoc oflags = V.castAs(); + NonLoc ocreateFlag = C.getSValBuilder() + .makeIntVal(Val_O_CREAT.getValue(), oflagsEx->getType()).castAs(); + SVal maskedFlagsUC = C.getSValBuilder().evalBinOpNN(state, BO_And, + oflags, ocreateFlag, + oflagsEx->getType()); + if (maskedFlagsUC.isUnknownOrUndef()) + return; + DefinedSVal maskedFlags = maskedFlagsUC.castAs(); + + // Check if maskedFlags is non-zero. + ProgramStateRef trueState, falseState; + std::tie(trueState, falseState) = state->assume(maskedFlags); + + // Only emit an error if the value of 'maskedFlags' is properly + // constrained; + if (!(trueState && !falseState)) + return; + + if (CE->getNumArgs() < MaxArgCount) { + return; + } + + // Now check mode when O_CREAT flag is set + // mode should not be higher than 0644 + Val_MODE = 0133; + const Expr *createModeEx = CE->getArg(CreateModeArgIndex); + const SVal CM = C.getSVal(createModeEx); + if (!CM.getAs()) { + return; + } + NonLoc createMode = CM.castAs(); + NonLoc createModeCheck = C.getSValBuilder(). + makeIntVal(Val_MODE.getValue(), createModeEx->getType()).castAs(); + + SVal maskedCreateMode = C.getSValBuilder().evalBinOpNN(state, BO_And, + createMode, createModeCheck, + createModeEx->getType()); + if (maskedCreateMode.isUnknownOrUndef()) + return; + DefinedSVal maskedCreateModeSVal = maskedCreateMode.castAs(); + + // Check if maskedFlags is non-zero. + ProgramStateRef t, f; + std::tie(t, f) = state->assume(maskedCreateModeSVal); + + // Only emit an error if the value of 'maskedFlags' is properly + // constrained; + if (t && !f) { + SmallString<256> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << "Open() system call with mode higher than 0644 is not allowed."; + ReportOpenBug(C, t, + SBuf.c_str(), + createModeEx->getSourceRange()); + } +} + +//===----------------------------------------------------------------------===// +// Registration. +//===----------------------------------------------------------------------===// + +#define REGISTER_CHECKER(CHECKERNAME) \ + void ento::register##CHECKERNAME(CheckerManager &mgr) { \ + mgr.registerChecker(); \ + } \ + \ + bool ento::shouldRegister##CHECKERNAME(const CheckerManager &mgr) { \ + return true; \ + } + +REGISTER_CHECKER(UnixAPIArgsChecker) diff --git a/clang/test/Analysis/memcpy_s.c b/clang/test/Analysis/memcpy_s.c new file mode 100644 index 0000000000000000000000000000000000000000..d976d0d02f45fb91658f00851452a80ae94bbb77 --- /dev/null +++ b/clang/test/Analysis/memcpy_s.c @@ -0,0 +1,53 @@ +#define __STDC_WANT_LIB_EXT1__ 1 +#include +#include +#include +#include +#include + +int memcpy_s(char *dst, int dstLen, char *src, int srcLen) { + return dstLen; +} + +// srcLen > dstStr's length, bug reported +void check1(int dstLen, int srcLen) { + char dstStr[dstLen]; + char srcStr[srcLen]; + memcpy_s(dstStr, sizeof(dstStr), srcStr, srcLen); +} + +// srcLen < dstStr's length, no bug reported +void check2() { + int dstLen = 20; + int srcLen = 10; + char dstStr[dstLen]; + char srcStr[srcLen]; + memcpy_s(dstStr, sizeof(dstStr), srcStr, srcLen); +} + +// srcLen > dstStr's length, bug reported +void check3() { + int dstLen = 10; + int srcLen = 20; + char dstStr[dstLen]; + char srcStr[srcLen]; + memcpy_s(dstStr, sizeof(dstStr), srcStr, srcLen); +} + +void check4() { + int dstLen = 20; + int srcLen = 10; + char dstStr[dstLen]; + char srcStr[srcLen]; + int offset = 15; + // srcLen > dstStr[offset]'s length, bug reported + memcpy_s(&dstStr[offset], srcLen, srcStr, srcLen); + offset = 5; + // srcLen < dstStr[offset]'s length, no bug reported + memcpy_s(&dstStr[offset], srcLen, srcStr, srcLen); +} + +// bug reported +void check5(char *dst, char *src, int dstLen, int srcLen) { + memcpy_s(dst, dstLen, src, srcLen); +} diff --git a/clang/test/Analysis/unix-api.c b/clang/test/Analysis/unix-api.c index 64ff3c0fccf426799cb8e4fc37aea39f376600b9..0f0c89c6b52f72c48ee4fd26c907b2c8b4bf5df1 100644 --- a/clang/test/Analysis/unix-api.c +++ b/clang/test/Analysis/unix-api.c @@ -4,6 +4,10 @@ #define O_RDONLY 0 #endif +#ifndef O_CREAT +#define O_CREAT 0100 +#endif + #ifndef NULL #define NULL ((void*) 0) #endif @@ -90,3 +94,11 @@ void open_8(const char *path) { if (fd > -1) close(fd); } + +void open_9(const char *path) { + int fd; + int mode = 0631; + fd = open(path, O_CREAT, mode); // expected-warning{{Open() system call with mode higher than 0644 is not allowed}} + if (fd > -1) + close(fd); +} \ No newline at end of file