boga95 updated this revision to Diff 218395. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59637/new/
https://reviews.llvm.org/D59637 Files: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp test/Analysis/taint-generic.c
Index: test/Analysis/taint-generic.c =================================================================== --- test/Analysis/taint-generic.c +++ test/Analysis/taint-generic.c @@ -338,3 +338,43 @@ if (i < rhs) *(volatile int *) 0; // no-warning } + + +// Test configuration +int mySource1(); +void mySource2(int*); +void myScanf(const char*, ...); +int myPropagator(int, int*); +int mySnprintf(char*, size_t, const char*, ...); +void mySink(int, int, int); + +void testConfigurationSources1() { + int x = mySource1(); + Buffer[x] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationSources2() { + int x; + mySource2(&x); + Buffer[x] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationSources3() { + int x, y; + myScanf("%d %d", &x, &y); + Buffer[y] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationPropagation() { + int x = mySource1(); + int y; + myPropagator(x, &y); + Buffer[y] = 1; // expected-warning {{Out of bound memory access }} +} + +void testConfigurationSinks() { + int x = mySource1(); + mySink(x, 1, 2); // expected-warning {{Untrusted data is passed to a user-defined sink}} + mySink(1, x, 2); // no-warning + mySink(1, 2, x); // expected-warning {{Untrusted data is passed to a user-defined sink}} +} Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -115,25 +115,38 @@ static Optional<SVal> getPointedToSVal(CheckerContext &C, const Expr *Arg); /// Check for CWE-134: Uncontrolled Format String. - static const char MsgUncontrolledFormatString[]; + static constexpr llvm::StringLiteral MsgUncontrolledFormatString = + "Untrusted data is used as a format string " + "(CWE-134: Uncontrolled Format String)"; bool checkUncontrolledFormatString(const CallExpr *CE, CheckerContext &C) const; /// Check for: /// CERT/STR02-C. "Sanitize data passed to complex subsystems" /// CWE-78, "Failure to Sanitize Data into an OS Command" - static const char MsgSanitizeSystemArgs[]; + static constexpr llvm::StringLiteral MsgSanitizeSystemArgs = + "Untrusted data is passed to a system call " + "(CERT/STR02-C. Sanitize data passed to complex subsystems)"; bool checkSystemCall(const CallExpr *CE, StringRef Name, CheckerContext &C) const; /// Check if tainted data is used as a buffer size ins strn.. functions, /// and allocators. - static const char MsgTaintedBufferSize[]; + static constexpr llvm::StringLiteral MsgTaintedBufferSize = + "Untrusted data is used to specify the buffer size " + "(CERT/STR31-C. Guarantee that storage for strings has sufficient space " + "for character data and the null terminator)"; bool checkTaintedBufferSize(const CallExpr *CE, const FunctionDecl *FDecl, CheckerContext &C) const; + /// Check if tainted data is used as a custom sink's parameter. + static constexpr llvm::StringLiteral MsgCustomSink = + "Untrusted data is passed to a user-defined sink"; + bool checkCustomSinks(const CallExpr *CE, StringRef Name, + CheckerContext &C) const; + /// Generate a report if the expression is tainted or points to tainted data. - bool generateReportIfTainted(const Expr *E, const char Msg[], + bool generateReportIfTainted(const Expr *E, const StringRef Msg, CheckerContext &C) const; /// A struct used to specify taint propagation rules for a function. @@ -175,7 +188,8 @@ /// Get the propagation rule for a given function. static TaintPropagationRule - getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name, + getTaintPropagationRule(const GenericTaintChecker *Checker, + const FunctionDecl *FDecl, StringRef Name, CheckerContext &C); void addSrcArg(unsigned A) { SrcArgs.push_back(A); } @@ -227,19 +241,6 @@ const unsigned GenericTaintChecker::ReturnValueIndex; const unsigned GenericTaintChecker::InvalidArgIndex; - -const char GenericTaintChecker::MsgUncontrolledFormatString[] = - "Untrusted data is used as a format string " - "(CWE-134: Uncontrolled Format String)"; - -const char GenericTaintChecker::MsgSanitizeSystemArgs[] = - "Untrusted data is passed to a system call " - "(CERT/STR02-C. Sanitize data passed to complex subsystems)"; - -const char GenericTaintChecker::MsgTaintedBufferSize[] = - "Untrusted data is used to specify the buffer size " - "(CERT/STR31-C. Guarantee that storage for strings has sufficient space " - "for character data and the null terminator)"; } // end of anonymous namespace using TaintConfig = GenericTaintChecker::TaintConfiguration; @@ -330,7 +331,8 @@ GenericTaintChecker::TaintPropagationRule GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( - const FunctionDecl *FDecl, StringRef Name, CheckerContext &C) { + const GenericTaintChecker *Checker, const FunctionDecl *FDecl, + StringRef Name, CheckerContext &C) { // TODO: Currently, we might lose precision here: we always mark a return // value as tainted even if it's just a pointer, pointing to tainted data. @@ -424,6 +426,10 @@ // or smart memory copy: // - memccpy - copying until hitting a special character. + auto It = Checker->CustomPropagations.find(Name); + if (It != Checker->CustomPropagations.end()) + return It->getValue(); + return TaintPropagationRule(); } @@ -464,7 +470,7 @@ // First, try generating a propagation rule for this function. TaintPropagationRule Rule = - TaintPropagationRule::getTaintPropagationRule(FDecl, Name, C); + TaintPropagationRule::getTaintPropagationRule(this, FDecl, Name, C); if (!Rule.isNull()) { State = Rule.process(CE, C); if (!State) @@ -536,6 +542,9 @@ if (checkTaintedBufferSize(CE, FDecl, C)) return true; + if (checkCustomSinks(CE, Name, C)) + return true; + return false; } @@ -573,7 +582,8 @@ bool IsTainted = true; for (unsigned ArgNum : SrcArgs) { if (ArgNum >= CE->getNumArgs()) - return State; + continue; + if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(ArgNum), State, C))) break; } @@ -601,8 +611,10 @@ continue; } + if (ArgNum >= CE->getNumArgs()) + continue; + // Mark the given argument. - assert(ArgNum < CE->getNumArgs()); State = State->add<TaintArgsOnPostVisit>(ArgNum); } @@ -700,7 +712,7 @@ } bool GenericTaintChecker::generateReportIfTainted(const Expr *E, - const char Msg[], + const StringRef Msg, CheckerContext &C) const { assert(E); @@ -756,9 +768,9 @@ .Case("execvP", 0) .Case("execve", 0) .Case("dlopen", 0) - .Default(UINT_MAX); + .Default(InvalidArgIndex); - if (ArgNum == UINT_MAX || CE->getNumArgs() < (ArgNum + 1)) + if (ArgNum == InvalidArgIndex || CE->getNumArgs() < (ArgNum + 1)) return false; return generateReportIfTainted(CE->getArg(ArgNum), MsgSanitizeSystemArgs, C); @@ -803,6 +815,24 @@ generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C); } +bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE, StringRef Name, + CheckerContext &C) const { + auto It = CustomSinks.find(Name); + if (It == CustomSinks.end()) + return false; + + const ArgVector &Args = It->getValue(); + for (unsigned ArgNum : Args) { + if (ArgNum >= CE->getNumArgs()) + continue; + + if (generateReportIfTainted(CE->getArg(ArgNum), MsgCustomSink, C)) + return true; + } + + return false; +} + void ento::registerGenericTaintChecker(CheckerManager &Mgr) { auto *Checker = Mgr.registerChecker<GenericTaintChecker>(); std::string Option{"Config"}; @@ -811,7 +841,7 @@ llvm::Optional<TaintConfig> Config = getConfiguration<TaintConfig>(Mgr, Checker, Option, ConfigFile); if (Config) - Checker->parseConfiguration(Mgr, Option, std::move(Config).getValue()); + Checker->parseConfiguration(Mgr, Option, std::move(Config.getValue())); } bool ento::shouldRegisterGenericTaintChecker(const LangOptions &LO) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits