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

Anna Zaks ganna at apple.com
Tue Dec 13 16:56:19 PST 2011


Author: zaks
Date: Tue Dec 13 18:56:18 2011
New Revision: 146536

URL: http://llvm.org/viewvc/llvm-project?rev=146536&view=rev
Log:
[analyzer] Treat stdin as a source of taint.

Some of the test cases do not currently work because the analyzer core
does not seem to call checkers for pre/post DeclRefExpr visits.
(Opened radar://10573500. To be fixed later on.)

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=146536&r1=146535&r2=146536&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Tue Dec 13 18:56:18 2011
@@ -24,7 +24,8 @@
 using namespace ento;
 
 namespace {
-class GenericTaintChecker : public Checker< check::PostStmt<CallExpr> > {
+class GenericTaintChecker : public Checker< check::PostStmt<CallExpr>,
+                                            check::PostStmt<DeclRefExpr> > {
 
   mutable llvm::OwningPtr<BugType> BT;
   void initBugType() const;
@@ -42,8 +43,12 @@
   void processFscanf(const CallExpr *CE, CheckerContext &C) const;
   void processRetTaint(const CallExpr *CE, CheckerContext &C) const;
 
+  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;
+  void checkPostStmt(const DeclRefExpr *DRE, CheckerContext &C) const;
 };
 }
 
@@ -77,18 +82,28 @@
   // Check and evaluate the call.
   if (evalFunction)
     (this->*evalFunction)(CE, C);
+}
 
+void GenericTaintChecker::checkPostStmt(const DeclRefExpr *DRE,
+                                       CheckerContext &C) const {
+  if (isStdin(DRE, C)) {
+    const ProgramState *NewState = C.getState()->addTaint(DRE);
+    C.addTransition(NewState);
+  }
 }
 
 SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C,
                                                   const Expr* Arg,
                                                   bool IssueWarning) const {
   const ProgramState *State = C.getState();
-  SVal AddrVal = State->getSVal(Arg->IgnoreParenCasts());
+  SVal AddrVal = State->getSVal(Arg->IgnoreParens());
 
-  // TODO: Taint is not going to propagate?
-  if (AddrVal.isUnknownOrUndef())
+  // TODO: Taint is not going to propagate? Should we ever peel off the casts
+  // IgnoreParenImpCasts()?
+  if (AddrVal.isUnknownOrUndef()) {
+    assert(State->getSVal(Arg->IgnoreParenImpCasts()).isUnknownOrUndef());
     return 0;
+  }
 
   Loc *AddrLoc = dyn_cast<Loc>(&AddrVal);
 
@@ -111,7 +126,6 @@
   return Val.getAsSymbol();
 }
 
-
 void GenericTaintChecker::processScanf(const CallExpr *CE,
                                        CheckerContext &C) const {
   const ProgramState *State = C.getState();
@@ -137,7 +151,7 @@
   assert(CE->getNumArgs() >= 2);
 
   // Check is the file descriptor is tainted.
-  if (!State->isTainted(CE->getArg(0)))
+  if (!State->isTainted(CE->getArg(0)) && !isStdin(CE->getArg(0), C))
     return;
 
   // All arguments except for the first two should get taint.
@@ -158,6 +172,30 @@
   C.addTransition(NewState);
 }
 
+bool GenericTaintChecker::isStdin(const Expr *E,
+                                  CheckerContext &C) const {
+  if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
+    return isStdin(DR, C);
+  return false;
+}
+
+bool GenericTaintChecker::isStdin(const DeclRefExpr *DR,
+                                  CheckerContext &C) const {
+  const VarDecl *D = dyn_cast_or_null<VarDecl>(DR->getDecl());
+  if (!D)
+    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;
+
+  return false;
+}
+
+
 void ento::registerGenericTaintChecker(CheckerManager &mgr) {
   mgr.registerChecker<GenericTaintChecker>();
 }

Modified: cfe/trunk/test/Analysis/taint-tester.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-tester.c?rev=146536&r1=146535&r2=146536&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/taint-tester.c (original)
+++ cfe/trunk/test/Analysis/taint-tester.c Tue Dec 13 18:56:18 2011
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1  -analyze -analyzer-checker=experimental.security.taint,debug.TaintTest -verify %s
 
+#include <stdarg.h>
+
 int scanf(const char *restrict format, ...);
 int getchar(void);
 
@@ -84,14 +86,10 @@
     }
 }
 
-struct _IO_FILE {
-  unsigned fakeField1;
-  char fakeField2;
-};
-typedef struct _IO_FILE FILE;
-extern struct _IO_FILE *stdin;
-extern struct _IO_FILE *stdout;
-extern struct _IO_FILE *stderr;
+typedef struct _FILE FILE;
+extern FILE *stdin;
+extern FILE *stdout;
+extern FILE *stderr;
 int fscanf(FILE *restrict stream, const char *restrict format, ...);
 int fprintf(FILE *stream, const char *format, ...);
 int fclose(FILE *stream);
@@ -102,13 +100,22 @@
   char s[80];
   int t;
 
+  // Check if stdin is treated as tainted.
+  fscanf(stdin, "%s %d", s, &t);
+  // Note, here, s is not tainted, but the data s points to is tainted.
+  char *ts = s;
+  char tss = s[0]; // expected-warning 1 {{tainted}}
+  int tt = t; // expected-warning 1 {{tainted}}
   if((fp=fopen("test", "w")) == 0) // expected-warning 3 {{tainted}}
     return 1;
-  // TODO: Have to mark stdin as tainted.
-  fscanf(stdin, "%s%d", s, &t);
-  fprintf(fp, "%s %d", s, t); // expected-warning 1 {{tainted}}
+  fprintf(fp, "%s %d", s, t); // expected-warning 2 {{tainted}}
   fclose(fp); // expected-warning 1 {{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 3 {{tainted}}
     return 1;
   fscanf(fp, "%s%d", s, &t); // expected-warning 1 {{tainted}}





More information about the cfe-commits mailing list