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

Reply via email to