[analyzer] Better stdin support.

llvm-svn: 146748

diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 7392872..8cdd89b 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -43,8 +43,9 @@ class GenericTaintChecker : public Checker< check::PostStmt<CallExpr>,
   void processFscanf(const CallExpr *CE, CheckerContext &C) const;
   void processRetTaint(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;
-  bool isStdin(const DeclRefExpr *E, CheckerContext &C) const;
 
 public:
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
@@ -97,13 +98,8 @@ SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C,
                                                   bool IssueWarning) const {
   const ProgramState *State = C.getState();
   SVal AddrVal = State->getSVal(Arg->IgnoreParens());
-
-  // TODO: Taint is not going to propagate? Should we ever peel off the casts
-  // IgnoreParenImpCasts()?
-  if (AddrVal.isUnknownOrUndef()) {
-    assert(State->getSVal(Arg->IgnoreParenImpCasts()).isUnknownOrUndef());
+  if (AddrVal.isUnknownOrUndef())
     return 0;
-  }
 
   Loc *AddrLoc = dyn_cast<Loc>(&AddrVal);
 
@@ -174,28 +170,38 @@ void GenericTaintChecker::processRetTaint(const CallExpr *CE,
 
 bool GenericTaintChecker::isStdin(const Expr *E,
                                   CheckerContext &C) const {
-  if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
-    return isStdin(DR, C);
-  return false;
-}
+  const ProgramState *State = C.getState();
+  SVal Val = State->getSVal(E);
 
-bool GenericTaintChecker::isStdin(const DeclRefExpr *DR,
-                                  CheckerContext &C) const {
-  const VarDecl *D = dyn_cast_or_null<VarDecl>(DR->getDecl());
-  if (!D)
+  // stdin is a pointer, so it would be a region.
+  const MemRegion *MemReg = Val.getAsRegion();
+
+  // The region should be symbolic, we do not know it's value.
+  const SymbolicRegion *SymReg = dyn_cast_or_null<SymbolicRegion>(MemReg);
+  if (!SymReg)
     return false;
 
-  D = D->getCanonicalDecl();
-  if ((D->getName().find("stdin") != StringRef::npos) && D->isExternC())
-    if (const PointerType * PtrTy =
-          dyn_cast<PointerType>(D->getType().getTypePtr()))
-      if (PtrTy->getPointeeType() == C.getASTContext().getFILEType())
-        return true;
+  // Get it's symbol and find the declaration region it's pointing to.
+  const SymbolRegionValue *Sm =dyn_cast<SymbolRegionValue>(SymReg->getSymbol());
+  if (!Sm)
+    return false;
+  const DeclRegion *DeclReg = dyn_cast_or_null<DeclRegion>(Sm->getRegion());
+  if (!DeclReg)
+    return false;
 
+  // This region corresponds to a declaration, find out if it's a global/extern
+  // variable named stdin with the proper type.
+  if (const VarDecl *D = dyn_cast_or_null<VarDecl>(DeclReg->getDecl())) {
+    D = D->getCanonicalDecl();
+    if ((D->getName().find("stdin") != StringRef::npos) && D->isExternC())
+        if (const PointerType * PtrTy =
+              dyn_cast<PointerType>(D->getType().getTypePtr()))
+          if (PtrTy->getPointeeType() == C.getASTContext().getFILEType())
+            return true;
+  }
   return false;
 }
 
-
 void ento::registerGenericTaintChecker(CheckerManager &mgr) {
   mgr.registerChecker<GenericTaintChecker>();
 }

diff --git a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
index 9ea8abd..76c25f2 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -653,6 +653,9 @@ bool ProgramState::scanReachableSymbols(const MemRegion * const *I,
 
 const ProgramState* ProgramState::addTaint(const Stmt *S,
                                            TaintTagType Kind) const {
+  if (const Expr *E = dyn_cast_or_null<Expr>(S))
+    S = E->IgnoreParens();
+
   SymbolRef Sym = getSVal(S).getAsSymbol();
   if (Sym)
     return addTaint(Sym, Kind);
@@ -679,6 +682,9 @@ const ProgramState* ProgramState::addTaint(SymbolRef Sym,
 }
 
 bool ProgramState::isTainted(const Stmt *S, TaintTagType Kind) const {
+  if (const Expr *E = dyn_cast_or_null<Expr>(S))
+    S = E->IgnoreParens();
+
   SVal val = getSVal(S);
   return isTainted(val, Kind);
 }

diff --git a/clang/test/Analysis/taint-tester.c b/clang/test/Analysis/taint-tester.c
index 476027f..8c964e4 100644 (file)
--- a/clang/test/Analysis/taint-tester.c
+++ b/clang/test/Analysis/taint-tester.c
@@ -111,10 +111,6 @@ int fscanfTest(void) {
   fprintf(fp, "%s %d", s, t); // expected-warning + {{tainted}}
   fclose(fp); // expected-warning + {{tainted}}
 
-  // Check if we propagate taint from stdin when it's used in an assignment.
-  FILE *pfstd = stdin;
-  fscanf(pfstd, "%s %d", s, &t); // TODO: This should be tainted as well.
-
   // Test fscanf and fopen.
   if((fp=fopen("test","r")) == 0) // expected-warning + {{tainted}}
     return 1;
@@ -122,3 +118,35 @@ int fscanfTest(void) {
   fprintf(stdout, "%s %d", s, t); // expected-warning + {{tainted}}
   return 0;
 }
+
+// Check if we propagate taint from stdin when it's used in an assignment.
+void stdinTest1() {
+  int i;
+  fscanf(stdin, "%d", &i);
+  int j = i; // expected-warning + {{tainted}}
+}
+void stdinTest2(FILE *pIn) {
+  FILE *p = stdin;
+  FILE *pp = p;
+  int ii;
+
+  fscanf(pp, "%d", &ii);
+  int jj = ii;// expected-warning + {{tainted}}
+
+  fscanf(p, "%d", &ii);
+  int jj2 = ii;// expected-warning + {{tainted}}
+
+  ii = 3;
+  int jj3 = ii;// no warning
+
+  p = pIn;
+  fscanf(p, "%d", &ii);
+  int jj4 = ii;// no warning
+}
+
+void stdinTest3() {
+  FILE **ppp = &stdin;
+  int iii;
+  fscanf(*ppp, "%d", &iii);
+  int jjj = iii;// expected-warning + {{tainted}}
+}