[analyzer] Add basic format string vulnerability checking.

We already have a more conservative check in the compiler (if the
format string is not a literal, we warn). Still adding it here for
completeness and since this check is stronger - only triggered if the
format string is tainted.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@147714 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

@@ -308,7 +308,7 @@ public:
   bool isTainted(const Stmt *S, const LocationContext *LCtx,
                  TaintTagType Kind = TaintTagGeneric) const;
   bool isTainted(SVal V, TaintTagType Kind = TaintTagGeneric) const;
-  bool isTainted(const SymExpr* Sym, TaintTagType Kind = TaintTagGeneric) const;
+  bool isTainted(SymbolRef Sym, TaintTagType Kind = TaintTagGeneric) const;
   bool isTainted(const MemRegion *Reg, TaintTagType Kind=TaintTagGeneric) const;
 
   //==---------------------------------------------------------------------==//

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

@@ -43,6 +43,15 @@ private:
   mutable llvm::OwningPtr<BugType> BT;
   void initBugType() const;
 
+  /// Add/propagate taint on a post visit.
+  void taintPost(const CallExpr *CE, CheckerContext &C) const;
+  /// Add/propagate taint on a pre visit.
+  void taintPre(const CallExpr *CE, CheckerContext &C) const;
+
+  /// Catch taint related bugs. Check if tainted data is passed to a system
+  /// call etc.
+  bool checkPre(const CallExpr *CE, CheckerContext &C) const;
+
   /// Given a pointer argument, get the symbol of the value it contains
   /// (points to).
   SymbolRef getPointedToSymbol(CheckerContext &C,
@@ -66,6 +75,10 @@ private:
   /// and thus, is tainted.
   bool isStdin(const Expr *E, CheckerContext &C) const;
 
+  /// Check for CWE-134: Uncontrolled Format String.
+  bool checkUncontrolledFormatString(const CallExpr *CE,
+                                     CheckerContext &C) const;
+
 public:
   static void *getTag() { static int Tag; return &Tag; }
 
@@ -94,13 +107,26 @@ namespace ento {
 
 inline void GenericTaintChecker::initBugType() const {
   if (!BT)
-    BT.reset(new BugType("Tainted data checking", "General"));
+    BT.reset(new BugType("Taint Analysis", "General"));
 }
 
 void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
                                        CheckerContext &C) const {
-  const ProgramState *State = C.getState();
+  // Check for errors first.
+  if (checkPre(CE, C))
+    return;
 
+  // Add taint second.
+  taintPre(CE, C);
+}
+
+void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
+                                        CheckerContext &C) const {
+  taintPost(CE, C);
+}
+
+void GenericTaintChecker::taintPre(const CallExpr *CE,
+                                   CheckerContext &C) const {
   // Set the evaluation function by switching on the callee name.
   StringRef Name = C.getCalleeName(CE);
   FnCheck evalFunction = llvm::StringSwitch<FnCheck>(Name)
@@ -111,6 +137,7 @@ void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
     .Default(0);
 
   // Check and evaluate the call.
+  const ProgramState *State = 0;
   if (evalFunction)
     State = (this->*evalFunction)(CE, C);
   if (!State)
@@ -119,10 +146,8 @@ void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
   C.addTransition(State);
 }
 
-void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
-                                        CheckerContext &C) const {
-  const ProgramState *State = C.getState();
-  
+void GenericTaintChecker::taintPost(const CallExpr *CE,
+                                    CheckerContext &C) const {
   // Define the attack surface.
   // Set the evaluation function by switching on the callee name.
   StringRef Name = C.getCalleeName(CE);
@@ -140,6 +165,7 @@ void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
 
   // If the callee isn't defined, it is not of security concern.
   // Check and evaluate the call.
+  const ProgramState *State = 0;
   if (evalFunction)
     State = (this->*evalFunction)(CE, C);
   if (!State)
@@ -150,6 +176,15 @@ void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
   C.addTransition(State);
 }
 
+bool GenericTaintChecker::checkPre(const CallExpr *CE, CheckerContext &C) const{
+
+  if (checkUncontrolledFormatString(CE, C))
+    return true;
+
+  StringRef Name = C.getCalleeName(CE);
+  return false;
+}
+
 SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C,
                                                   const Expr* Arg,
                                                   bool IssueWarning) const {
@@ -301,6 +336,57 @@ bool GenericTaintChecker::isStdin(const Expr *E,
   return false;
 }
 
+static bool getPrintfFormatArgumentNum(const CallExpr *CE,
+                                       const CheckerContext &C,
+                                       unsigned int &ArgNum) {
+  // Find if the function contains a format string argument.
+  // Handles: fprintf, printf, sprintf, snprintf, vfprintf, vprintf, vsprintf,
+  // vsnprintf, syslog, custom annotated functions.
+  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  if (!FDecl)
+    return false;
+  for (specific_attr_iterator<FormatAttr>
+         i = FDecl->specific_attr_begin<FormatAttr>(),
+         e = FDecl->specific_attr_end<FormatAttr>(); i != e ; ++i) {
+
+    const FormatAttr *Format = *i;
+    ArgNum = Format->getFormatIdx() - 1;
+    if ((Format->getType() == "printf") && CE->getNumArgs() > ArgNum)
+      return true;
+  }
+
+  // Or if a function is named setproctitle (this is a heuristic).
+  if (C.getCalleeName(CE).find("setproctitle") != StringRef::npos) {
+    ArgNum = 0;
+    return true;
+  }
+
+  return false;
+}
+
+bool GenericTaintChecker::checkUncontrolledFormatString(const CallExpr *CE,
+                                                        CheckerContext &C) const{
+  // Check if the function contains a format string argument.
+  unsigned int ArgNum = 0;
+  if (!getPrintfFormatArgumentNum(CE, C, ArgNum))
+    return false;
+
+  // If either the format string content or the pointer itself are tainted, warn.
+  const ProgramState *State = C.getState();
+  const Expr *Arg = CE->getArg(ArgNum);
+  if (State->isTainted(getPointedToSymbol(C, Arg, false)) ||
+      State->isTainted(Arg, C.getLocationContext()))
+    if (ExplodedNode *N = C.addTransition()) {
+      initBugType();
+      BugReport *report = new BugReport(*BT,
+        "Tainted format string (CWE-134: Uncontrolled Format String)", N);
+      report->addRange(Arg->getSourceRange());
+      C.EmitReport(report);
+      return true;
+    }
+  return false;
+}
+
 void ento::registerGenericTaintChecker(CheckerManager &mgr) {
   mgr.registerChecker<GenericTaintChecker>();
 }

lib/StaticAnalyzer/Core/ProgramState.cpp

@@ -640,7 +640,7 @@ bool ProgramState::isTainted(const MemRegion *Reg, TaintTagType K) const {
   return false;
 }
 
-bool ProgramState::isTainted(const SymExpr* Sym, TaintTagType Kind) const {
+bool ProgramState::isTainted(SymbolRef Sym, TaintTagType Kind) const {
   if (!Sym)
     return false;
   

test/Analysis/taint-generic.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1  -analyze -analyzer-checker=experimental.security.taint,experimental.security.ArrayBoundV2 -verify %s
+// RUN: %clang_cc1  -analyze -analyzer-checker=experimental.security.taint,experimental.security.ArrayBoundV2 -Wno-format-security -verify %s
 
 int scanf(const char *restrict format, ...);
 int getchar(void);
@@ -46,3 +46,17 @@ void bufferGetchar(int x) {
   int m = getchar();
   Buffer[m] = 1;  //expected-warning {{Out of bound memory access }}
 }
+
+typedef struct _FILE FILE;
+extern FILE *stdin;
+int fscanf(FILE *restrict stream, const char *restrict format, ...);
+int sprintf(char *str, const char *format, ...);
+void setproctitle(const char *fmt, ...);
+
+void testUncontrolledFormatString() {
+  char s[80];
+  fscanf(stdin, "%s", s);
+  char buf[128];
+  sprintf(buf,s); // expected-warning {{Uncontrolled Format String}}
+  setproctitle(s, 3); // expected-warning {{Uncontrolled Format String}}
+}