Move prop-sink branch to monorepo.
authorGabor Borsik <gabor.borsik@gmail.com>
Sun, 8 Sep 2019 19:23:43 +0000 (19:23 +0000)
committerGabor Borsik <gabor.borsik@gmail.com>
Sun, 8 Sep 2019 19:23:43 +0000 (19:23 +0000)
llvm-svn: 371342

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/test/Analysis/taint-generic.c

index 83fbb19..161ce54 100644 (file)
@@ -115,27 +115,44 @@ private:
   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, StringRef Msg,
                                CheckerContext &C) const;
 
+  struct TaintPropagationRule;
+  using NameRuleMap = llvm::StringMap<TaintPropagationRule>;
+  using NameArgMap = llvm::StringMap<ArgVector>;
+
   /// A struct used to specify taint propagation rules for a function.
   ///
   /// If any of the possible taint source arguments is tainted, all of the
@@ -175,7 +192,8 @@ private:
 
     /// Get the propagation rule for a given function.
     static TaintPropagationRule
-    getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name,
+    getTaintPropagationRule(const NameRuleMap &CustomPropagations,
+                            const FunctionDecl *FDecl, StringRef Name,
                             CheckerContext &C);
 
     void addSrcArg(unsigned A) { SrcArgs.push_back(A); }
@@ -211,9 +229,6 @@ private:
                            CheckerContext &C);
   };
 
-  using NameRuleMap = llvm::StringMap<TaintPropagationRule>;
-  using NameArgMap = llvm::StringMap<ArgVector>;
-
   /// Defines a map between the propagation function's name and
   /// TaintPropagationRule.
   NameRuleMap CustomPropagations;
@@ -228,18 +243,11 @@ private:
 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)";
+// FIXME: these lines can be removed in C++17
+constexpr llvm::StringLiteral GenericTaintChecker::MsgUncontrolledFormatString;
+constexpr llvm::StringLiteral GenericTaintChecker::MsgSanitizeSystemArgs;
+constexpr llvm::StringLiteral GenericTaintChecker::MsgTaintedBufferSize;
+constexpr llvm::StringLiteral GenericTaintChecker::MsgCustomSink;
 } // end of anonymous namespace
 
 using TaintConfig = GenericTaintChecker::TaintConfiguration;
@@ -330,7 +338,8 @@ void GenericTaintChecker::parseConfiguration(CheckerManager &Mgr,
 
 GenericTaintChecker::TaintPropagationRule
 GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
-    const FunctionDecl *FDecl, StringRef Name, CheckerContext &C) {
+    const NameRuleMap &CustomPropagations, 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 +433,10 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
   // or smart memory copy:
   // - memccpy - copying until hitting a special character.
 
+  auto It = CustomPropagations.find(Name);
+  if (It != CustomPropagations.end())
+    return It->getValue();
+
   return TaintPropagationRule();
 }
 
@@ -463,8 +476,8 @@ void GenericTaintChecker::addSourcesPre(const CallExpr *CE,
     return;
 
   // First, try generating a propagation rule for this function.
-  TaintPropagationRule Rule =
-      TaintPropagationRule::getTaintPropagationRule(FDecl, Name, C);
+  TaintPropagationRule Rule = TaintPropagationRule::getTaintPropagationRule(
+      this->CustomPropagations, FDecl, Name, C);
   if (!Rule.isNull()) {
     State = Rule.process(CE, C);
     if (!State)
@@ -536,6 +549,9 @@ bool GenericTaintChecker::checkPre(const CallExpr *CE,
   if (checkTaintedBufferSize(CE, FDecl, C))
     return true;
 
+  if (checkCustomSinks(CE, Name, C))
+    return true;
+
   return false;
 }
 
@@ -573,7 +589,8 @@ GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE,
   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 +618,10 @@ GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE,
       continue;
     }
 
+    if (ArgNum >= CE->getNumArgs())
+      continue;
+
     // Mark the given argument.
-    assert(ArgNum < CE->getNumArgs());
     State = State->add<TaintArgsOnPostVisit>(ArgNum);
   }
 
@@ -699,8 +718,7 @@ static bool getPrintfFormatArgumentNum(const CallExpr *CE,
   return false;
 }
 
-bool GenericTaintChecker::generateReportIfTainted(const Expr *E,
-                                                  const char Msg[],
+bool GenericTaintChecker::generateReportIfTainted(const Expr *E, StringRef Msg,
                                                   CheckerContext &C) const {
   assert(E);
 
@@ -756,9 +774,9 @@ bool GenericTaintChecker::checkSystemCall(const CallExpr *CE, StringRef Name,
                         .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 +821,24 @@ bool GenericTaintChecker::checkTaintedBufferSize(const CallExpr *CE,
          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 GenericTaintChecker::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"};
index a6aae22..4d933e7 100644 (file)
@@ -338,3 +338,45 @@ void constraintManagerShouldTreatAsOpaque(int rhs) {
   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@-1 {{Untrusted data is passed to a user-defined sink}}
+  mySink(1, x, 2); // no-warning
+  mySink(1, 2, x);
+  // expected-warning@-1 {{Untrusted data is passed to a user-defined sink}}
+}