diff options
| -rw-r--r-- | lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp | 50 | ||||
| -rw-r--r-- | lib/StaticAnalyzer/Core/ProgramState.cpp | 6 | ||||
| -rw-r--r-- | test/Analysis/taint-tester.c | 36 |
3 files changed, 66 insertions, 26 deletions
diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 739287284b..8cdd89b1c4 100644 --- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/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/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp index 9ea8abd952..76c25f2226 100644 --- a/lib/StaticAnalyzer/Core/ProgramState.cpp +++ b/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/test/Analysis/taint-tester.c b/test/Analysis/taint-tester.c index 476027f31b..8c964e4ae2 100644 --- a/test/Analysis/taint-tester.c +++ b/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}} +} |