[analyzer] Warn when non pointer arguments are passed to scanf (only when running taint checker).

There is an open radar to implement better scanf checking as a Sema warning. However, a bit of redundancy is fine in this case.

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

3 files changed, 40 insertions, 5 deletions

diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 0fcfdf8ff5..c8e54efb67 100644
--- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -26,7 +26,14 @@ using namespace ento;
 namespace {
 class GenericTaintChecker : public Checker< check::PostStmt<CallExpr> > {
 
-  mutable llvm::OwningPtr<BuiltinBug> BT;
+  mutable llvm::OwningPtr<BugType> BT;
+  void initBugType() const;
+
+  /// Given a pointer argument, get the symbol of the value it contains
+  /// (points to).
+  SymbolRef getPointedToSymbol(CheckerContext &C,
+                               const Expr* Arg,
+                               bool IssueWarning = true) const;
 
   /// Functions defining the attacke surface.
   typedef void (GenericTaintChecker::*FnCheck)(const CallExpr *,
@@ -39,6 +46,11 @@ public:
 };
 }
 
+inline void GenericTaintChecker::initBugType() const {
+  if (!BT)
+    BT.reset(new BugType("Tainted data checking", "General"));
+}
+
 void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
                                         CheckerContext &C) const {
   if (!C.getState())
@@ -59,10 +71,29 @@ void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
     (this->*evalFunction)(CE, C);
 
 }
-static SymbolRef getPointedToSymbol(const ProgramState *State,
-                                    const Expr* Arg) {
+
+SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C,
+                                                  const Expr* Arg,
+                                                  bool IssueWarning) const {
+  const ProgramState *State = C.getState();
   SVal AddrVal = State->getSVal(Arg->IgnoreParenCasts());
   Loc *AddrLoc = dyn_cast<Loc>(&AddrVal);
+
+  if (!AddrLoc && !IssueWarning)
+    return 0;
+
+  // If the Expr is not a location, issue a warning.
+  if (!AddrLoc) {
+    assert(IssueWarning);
+    if (ExplodedNode *N = C.generateSink(State)) {
+      initBugType();
+      BugReport *report = new BugReport(*BT, "Pointer argument is expected.",N);
+      report->addRange(Arg->getSourceRange());
+      C.EmitReport(report);
+    }
+    return 0;
+  }
+
   SVal Val = State->getSVal(*AddrLoc);
   return Val.getAsSymbol();
 }
@@ -78,7 +109,7 @@ void GenericTaintChecker::processScanf(const CallExpr *CE,
     // The arguments are pointer arguments. The data they are pointing at is
     // tainted after the call.
     const Expr* Arg = CE->getArg(i);
-    SymbolRef Sym = getPointedToSymbol(State, Arg);
+    SymbolRef Sym = getPointedToSymbol(C, Arg);
     if (Sym)
       State = State->addTaint(Sym);
   }
diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp
index db2097c16f..778a0bf97d 100644
--- a/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -177,7 +177,6 @@ SVal SValBuilder::generateUnknownVal(const ProgramState *State,
       symLHS = LHS.getAsSymExpr();
       return makeNonLoc(symLHS, Op, rInt->getValue(), ResultTy);
     }
-    // TODO: Handle the case when lhs is ConcreteInt.
 
     symLHS = LHS.getAsSymExpr();
     symRHS = RHS.getAsSymExpr();
diff --git a/test/Analysis/taint-generic.c b/test/Analysis/taint-generic.c
index 2e3def3670..54229937d1 100644
--- a/test/Analysis/taint-generic.c
+++ b/test/Analysis/taint-generic.c
@@ -26,3 +26,8 @@ void bufferScanfArithmetic2(int x) {
   int m = (n + 3) * x;
   Buffer[m] = 1; // expected-warning {{Out of bound memory access }}
 }
+
+void scanfArg() {
+  int t;
+  scanf("%d", t); // expected-warning {{Pointer argument is expected}}
+}