[cfe-commits] r148844 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp test/Analysis/taint-tester.c test/Analysis/taint-tester.cpp

Anna Zaks ganna at apple.com
Tue Jan 24 11:32:25 PST 2012


Author: zaks
Date: Tue Jan 24 13:32:25 2012
New Revision: 148844

URL: http://llvm.org/viewvc/llvm-project?rev=148844&view=rev
Log:
[analyzer] Add more C taint sources/sinks.

Added:
    cfe/trunk/test/Analysis/taint-tester.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
    cfe/trunk/test/Analysis/taint-tester.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=148844&r1=148843&r2=148844&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Tue Jan 24 13:32:25 2012
@@ -38,8 +38,9 @@
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
 
 private:
-  static const unsigned ReturnValueIndex = UINT_MAX;
-  static const unsigned InvalidArgIndex = UINT_MAX - 1;
+  static const unsigned InvalidArgIndex = UINT_MAX;
+  /// Denotes the return vale.
+  static const unsigned ReturnValueIndex = UINT_MAX - 1;
 
   mutable llvm::OwningPtr<BugType> BT;
   inline void initBugType() const {
@@ -60,18 +61,14 @@
   /// \brief Add taint sources on a post visit.
   void addSourcesPost(const CallExpr *CE, CheckerContext &C) const;
 
+  /// Check if the region the expression evaluates to is the standard input,
+  /// and thus, is tainted.
+  static bool isStdin(const Expr *E, CheckerContext &C);
+
   /// \brief Given a pointer argument, get the symbol of the value it contains
   /// (points to).
   static SymbolRef getPointedToSymbol(CheckerContext &C, const Expr *Arg);
 
-  static inline bool isTaintedOrPointsToTainted(const Expr *E,
-                                                const ProgramState *State,
-                                                CheckerContext &C) {
-    return (State->isTainted(E, C.getLocationContext()) ||
-            (E->getType().getTypePtr()->isPointerType() &&
-             State->isTainted(getPointedToSymbol(C, E))));
-  }
-
   /// Functions defining the attack surface.
   typedef const ProgramState *(GenericTaintChecker::*FnCheck)(const CallExpr *,
                                                        CheckerContext &C) const;
@@ -82,10 +79,6 @@
   /// Taint the scanned input if the file is tainted.
   const ProgramState *preFscanf(const CallExpr *CE, CheckerContext &C) const;
 
-  /// Check if the region the expression evaluates to is the standard input,
-  /// and thus, is tainted.
-  bool isStdin(const Expr *E, CheckerContext &C) const;
-
   /// Check for CWE-134: Uncontrolled Format String.
   static const char MsgUncontrolledFormatString[];
   bool checkUncontrolledFormatString(const CallExpr *CE,
@@ -162,6 +155,14 @@
                         DstArgs.end(), ArgNum) != DstArgs.end());
     }
 
+    static inline bool isTaintedOrPointsToTainted(const Expr *E,
+                                                  const ProgramState *State,
+                                                  CheckerContext &C) {
+      return (State->isTainted(E, C.getLocationContext()) || isStdin(E, C) ||
+              (E->getType().getTypePtr()->isPointerType() &&
+               State->isTainted(getPointedToSymbol(C, E))));
+    }
+
     /// \brief Pre-process a function which propagates taint according to the
     /// taint rule.
     const ProgramState *process(const CallExpr *CE, CheckerContext &C) const;
@@ -203,13 +204,29 @@
                                                      const FunctionDecl *FDecl,
                                                      StringRef Name,
                                                      CheckerContext &C) {
+  // TODO: Currently, we might loose precision here: we always mark a return
+  // value as tainted even if it's just a pointer, pointing to tainted data.
+
   // Check for exact name match for functions without builtin substitutes.
   TaintPropagationRule Rule = llvm::StringSwitch<TaintPropagationRule>(Name)
     .Case("atoi", TaintPropagationRule(0, ReturnValueIndex))
     .Case("atol", TaintPropagationRule(0, ReturnValueIndex))
     .Case("atoll", TaintPropagationRule(0, ReturnValueIndex))
+    .Case("getc", TaintPropagationRule(0, ReturnValueIndex))
+    .Case("fgetc", TaintPropagationRule(0, ReturnValueIndex))
+    .Case("getc_unlocked", TaintPropagationRule(0, ReturnValueIndex))
+    .Case("getw", TaintPropagationRule(0, ReturnValueIndex))
+    .Case("toupper", TaintPropagationRule(0, ReturnValueIndex))
+    .Case("tolower", TaintPropagationRule(0, ReturnValueIndex))
+    .Case("strchr", TaintPropagationRule(0, ReturnValueIndex))
+    .Case("strrchr", TaintPropagationRule(0, ReturnValueIndex))
     .Case("read", TaintPropagationRule(0, 2, 1, true))
     .Case("pread", TaintPropagationRule(InvalidArgIndex, 1, true))
+    .Case("gets", TaintPropagationRule(InvalidArgIndex, 0, true))
+    .Case("fgets", TaintPropagationRule(2, 0, true))
+    .Case("getline", TaintPropagationRule(2, 0))
+    .Case("getdelim", TaintPropagationRule(3, 0))
+    .Case("fgetln", TaintPropagationRule(0, ReturnValueIndex))
     .Default(TaintPropagationRule());
 
   if (!Rule.isNull())
@@ -317,6 +334,9 @@
   // arguments which should be tainted after the function returns. These are
   // stored in the state as TaintArgsOnPostVisit set.
   llvm::ImmutableSet<unsigned> TaintArgs = State->get<TaintArgsOnPostVisit>();
+  if (TaintArgs.isEmpty())
+    return false;
+
   for (llvm::ImmutableSet<unsigned>::iterator
          I = TaintArgs.begin(), E = TaintArgs.end(); I != E; ++I) {
     unsigned ArgNum  = *I;
@@ -356,10 +376,13 @@
     .Case("scanf", &GenericTaintChecker::postScanf)
     // TODO: Add support for vfscanf & family.
     .Case("getchar", &GenericTaintChecker::postRetTaint)
+    .Case("getchar_unlocked", &GenericTaintChecker::postRetTaint)
     .Case("getenv", &GenericTaintChecker::postRetTaint)
     .Case("fopen", &GenericTaintChecker::postRetTaint)
     .Case("fdopen", &GenericTaintChecker::postRetTaint)
     .Case("freopen", &GenericTaintChecker::postRetTaint)
+    .Case("getch", &GenericTaintChecker::postRetTaint)
+    .Case("wgetch", &GenericTaintChecker::postRetTaint)
     .Case("socket", &GenericTaintChecker::postSocket)
     .Default(0);
 
@@ -428,18 +451,14 @@
       for (unsigned int i = 0; i < CE->getNumArgs(); ++i) {
         if (isDestinationArgument(i))
           continue;
-        if ((IsTainted =
-               GenericTaintChecker::isTaintedOrPointsToTainted(CE->getArg(i),
-                                                               State, C)))
+        if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(i), State, C)))
           break;
       }
       break;
     }
 
     assert(ArgNum < CE->getNumArgs());
-    if ((IsTainted =
-           GenericTaintChecker::isTaintedOrPointsToTainted(CE->getArg(ArgNum),
-                                                           State, C)))
+    if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(ArgNum), State, C)))
       break;
   }
   if (!IsTainted)
@@ -541,8 +560,7 @@
   return C.getState()->addTaint(CE, C.getLocationContext());
 }
 
-bool GenericTaintChecker::isStdin(const Expr *E,
-                                  CheckerContext &C) const {
+bool GenericTaintChecker::isStdin(const Expr *E, CheckerContext &C) {
   const ProgramState *State = C.getState();
   SVal Val = State->getSVal(E, C.getLocationContext());
 
@@ -642,6 +660,9 @@
 bool GenericTaintChecker::checkSystemCall(const CallExpr *CE,
                                           StringRef Name,
                                           CheckerContext &C) const {
+  // TODO: It might make sense to run this check on demand. In some cases, 
+  // we should check if the environment has been cleansed here. We also might 
+  // need to know if the user was reset before these calls(seteuid).
   unsigned ArgNum = llvm::StringSwitch<unsigned>(Name)
     .Case("system", 0)
     .Case("popen", 0)
@@ -651,6 +672,8 @@
     .Case("execv", 0)
     .Case("execvp", 0)
     .Case("execvP", 0)
+    .Case("execve", 0)
+    .Case("dlopen", 0)
     .Default(UINT_MAX);
 
   if (ArgNum == UINT_MAX)

Modified: cfe/trunk/test/Analysis/taint-tester.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-tester.c?rev=148844&r1=148843&r2=148844&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/taint-tester.c (original)
+++ cfe/trunk/test/Analysis/taint-tester.c Tue Jan 24 13:32:25 2012
@@ -4,6 +4,7 @@
 
 int scanf(const char *restrict format, ...);
 int getchar(void);
+typedef __typeof(sizeof(int)) size_t;
 
 #define BUFSIZE 10
 int Buffer[BUFSIZE];
@@ -160,6 +161,26 @@
   int j = i; // expected-warning + {{tainted}}
 }
 
+int getw(FILE *);
+void getwTest() {
+  int i = getw(stdin); // expected-warning + {{tainted}}
+}
+
+typedef long ssize_t;
+ssize_t getline(char ** __restrict, size_t * __restrict, FILE * __restrict);
+int  printf(const char * __restrict, ...);
+void free(void *ptr);
+void getlineTest(void) {
+  FILE *fp;
+  char *line = 0;
+  size_t len = 0;
+  ssize_t read;
+  while ((read = getline(&line, &len, stdin)) != -1) {
+    printf("%s", line); // expected-warning + {{tainted}}
+  }
+  free(line); // expected-warning + {{tainted}}
+}
+
 // Test propagation functions - the ones that propagate taint from arguments to
 // return value, ptr arguments.
 

Added: cfe/trunk/test/Analysis/taint-tester.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-tester.cpp?rev=148844&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/taint-tester.cpp (added)
+++ cfe/trunk/test/Analysis/taint-tester.cpp Tue Jan 24 13:32:25 2012
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1  -analyze -analyzer-checker=experimental.security.taint,debug.TaintTest %s -verify
+
+typedef struct _FILE FILE;
+typedef __typeof(sizeof(int)) size_t;
+extern FILE *stdin;
+typedef long ssize_t;
+ssize_t getline(char ** __restrict, size_t * __restrict, FILE * __restrict);
+int  printf(const char * __restrict, ...);
+void free(void *ptr);
+
+struct GetLineTestStruct {
+  ssize_t getline(char ** __restrict, size_t * __restrict, FILE * __restrict);
+};
+
+void getlineTest(void) {
+  FILE *fp;
+  char *line = 0;
+  size_t len = 0;
+  ssize_t read;
+  struct GetLineTestStruct T;
+
+  while ((read = T.getline(&line, &len, stdin)) != -1) {
+    printf("%s", line); // no warning
+  }
+  free(line);
+}





More information about the cfe-commits mailing list