From 9ffbe243cca46082b4a59b5c3be454ab0c455378 Mon Sep 17 00:00:00 2001 From: Anna Zaks Date: Sat, 17 Dec 2011 00:26:34 +0000 Subject: [PATCH] [analyzer] Add support for taint flowing through a function (atoi). Check if the input parameters are tainted (or point to tainted data) on a checkPreStmt. If the output should be tainted, record it in the state. On post visit (checkPostStmt), use the state to make decisions (in addition to the existing logic). Use this logic for atoi and fscanf. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@146793 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/GenericTaintChecker.cpp | 183 ++++++++++++++---- test/Analysis/taint-tester.c | 16 ++ 2 files changed, 156 insertions(+), 43 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 8cdd89b1c4..f09d670c1f 100644 --- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -18,6 +18,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" using namespace clang; @@ -25,72 +26,128 @@ using namespace ento; namespace { class GenericTaintChecker : public Checker< check::PostStmt, - check::PostStmt > { + check::PreStmt > { +public: + enum TaintOnPreVisitKind { + /// No taint propagates from pre-visit to post-visit. + PrevisitNone = 0, + /// Based on the pre-visit, the return argument of the call + /// should be tainted. + PrevisitTaintRet = 1, + /// Based on the pre-visit, the call can taint values through it's + /// pointer/reference arguments. + PrevisitTaintArgs = 2 + }; +private: mutable llvm::OwningPtr 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, + const Expr *Arg, bool IssueWarning = true) const; - /// Functions defining the attacke surface. - typedef void (GenericTaintChecker::*FnCheck)(const CallExpr *, - CheckerContext &C) const; - void processScanf(const CallExpr *CE, CheckerContext &C) const; - void processFscanf(const CallExpr *CE, CheckerContext &C) const; - void processRetTaint(const CallExpr *CE, CheckerContext &C) const; + /// Functions defining the attack surface. + typedef const ProgramState *(GenericTaintChecker::*FnCheck)(const CallExpr *, + CheckerContext &C) const; + const ProgramState *postScanf(const CallExpr *CE, CheckerContext &C) const; + const ProgramState *postFscanf(const CallExpr *CE, CheckerContext &C) const; + const ProgramState *postRetTaint(const CallExpr *CE, CheckerContext &C) const; + const ProgramState *postDefault(const CallExpr *CE, CheckerContext &C) const; + + /// Taint the scanned input if the file is tainted. + const ProgramState *preFscanf(const CallExpr *CE, CheckerContext &C) const; + /// Taint if any of the arguments are tainted. + const ProgramState *preAnyArgs(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; public: + static void *getTag() { static int Tag; return &Tag; } + void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; void checkPostStmt(const DeclRefExpr *DRE, CheckerContext &C) const; + + void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; + }; } +/// Definitions for the checker specific state. +namespace { struct TaintOnPreVisit {};} +namespace clang { +namespace ento { + /// A flag which is used to pass information from call pre-visit instruction + /// to the call post-visit. The value is an unsigned, which takes on values + /// of the TaintOnPreVisitKind enumeration. + template<> + struct ProgramStateTrait : + public ProgramStatePartialTrait { + static void *GDMIndex() { return GenericTaintChecker::getTag(); } + }; +} +} + 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()) - return; +void GenericTaintChecker::checkPreStmt(const CallExpr *CE, + CheckerContext &C) const { + const ProgramState *State = C.getState(); + // Set the evaluation function by switching on the callee name. StringRef Name = C.getCalleeName(CE); + FnCheck evalFunction = llvm::StringSwitch(Name) + .Case("fscanf", &GenericTaintChecker::preFscanf) + .Case("atoi", &GenericTaintChecker::preAnyArgs) + .Case("atol", &GenericTaintChecker::preAnyArgs) + .Case("atoll", &GenericTaintChecker::preAnyArgs) + .Default(0); + + // Check and evaluate the call. + if (evalFunction) + State = (this->*evalFunction)(CE, C); + if (!State) + return; + + C.addTransition(State); +} + +void GenericTaintChecker::checkPostStmt(const CallExpr *CE, + CheckerContext &C) const { + const ProgramState *State = C.getState(); // Define the attack surface. // Set the evaluation function by switching on the callee name. + StringRef Name = C.getCalleeName(CE); FnCheck evalFunction = llvm::StringSwitch(Name) - .Case("scanf", &GenericTaintChecker::processScanf) - .Case("fscanf", &GenericTaintChecker::processFscanf) - .Case("sscanf", &GenericTaintChecker::processFscanf) + .Case("scanf", &GenericTaintChecker::postScanf) + .Case("fscanf", &GenericTaintChecker::postFscanf) + .Case("sscanf", &GenericTaintChecker::postFscanf) // TODO: Add support for vfscanf & family. - .Case("getchar", &GenericTaintChecker::processRetTaint) - .Case("getenv", &GenericTaintChecker::processRetTaint) - .Case("fopen", &GenericTaintChecker::processRetTaint) - .Case("fdopen", &GenericTaintChecker::processRetTaint) - .Case("freopen", &GenericTaintChecker::processRetTaint) - .Default(NULL); + .Case("getchar", &GenericTaintChecker::postRetTaint) + .Case("getenv", &GenericTaintChecker::postRetTaint) + .Case("fopen", &GenericTaintChecker::postRetTaint) + .Case("fdopen", &GenericTaintChecker::postRetTaint) + .Case("freopen", &GenericTaintChecker::postRetTaint) + .Default(&GenericTaintChecker::postDefault); // If the callee isn't defined, it is not of security concern. // Check and evaluate the call. if (evalFunction) - (this->*evalFunction)(CE, C); -} + State = (this->*evalFunction)(CE, C); + if (!State) + return; -void GenericTaintChecker::checkPostStmt(const DeclRefExpr *DRE, - CheckerContext &C) const { - if (isStdin(DRE, C)) { - const ProgramState *NewState = C.getState()->addTaint(DRE); - C.addTransition(NewState); - } + assert(State->get() == PrevisitNone && + "State has to be cleared."); + C.addTransition(State); } SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C, @@ -122,8 +179,45 @@ SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C, return Val.getAsSymbol(); } -void GenericTaintChecker::processScanf(const CallExpr *CE, - CheckerContext &C) const { +const ProgramState *GenericTaintChecker::preFscanf(const CallExpr *CE, + CheckerContext &C) const { + assert(CE->getNumArgs() >= 2); + const ProgramState *State = C.getState(); + + // Check is the file descriptor is tainted. + if (State->isTainted(CE->getArg(0)) || isStdin(CE->getArg(0), C)) + return State->set(PrevisitTaintArgs); + return 0; +} + +// If any other arguments are tainted, mark state as tainted on pre-visit. +const ProgramState * GenericTaintChecker::preAnyArgs(const CallExpr *CE, + CheckerContext &C) const { + for (unsigned int i = 0; i < CE->getNumArgs(); ++i) { + const ProgramState *State = C.getState(); + const Expr *Arg = CE->getArg(i); + if (State->isTainted(Arg) || State->isTainted(getPointedToSymbol(C, Arg))) + return State = State->set(PrevisitTaintRet); + } + return 0; +} + +const ProgramState *GenericTaintChecker::postDefault(const CallExpr *CE, + CheckerContext &C) const { + const ProgramState *State = C.getState(); + + // Check if we know that the result needs to be tainted based on the + // pre-visit analysis. + if (State->get() == PrevisitTaintRet) { + State = State->addTaint(CE); + return State->set(PrevisitNone); + } + + return 0; +} + +const ProgramState *GenericTaintChecker::postScanf(const CallExpr *CE, + CheckerContext &C) const { const ProgramState *State = C.getState(); assert(CE->getNumArgs() >= 2); SVal x = State->getSVal(CE->getArg(1)); @@ -132,23 +226,27 @@ 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(C, Arg); + SymbolRef Sym = getPointedToSymbol(C, Arg); if (Sym) State = State->addTaint(Sym); } - C.addTransition(State); + return State; } /// If argument 0 (file descriptor) is tainted, all arguments except for arg 0 /// and arg 1 should get taint. -void GenericTaintChecker::processFscanf(const CallExpr *CE, - CheckerContext &C) const { +const ProgramState *GenericTaintChecker::postFscanf(const CallExpr *CE, + CheckerContext &C) const { const ProgramState *State = C.getState(); assert(CE->getNumArgs() >= 2); - // Check is the file descriptor is tainted. - if (!State->isTainted(CE->getArg(0)) && !isStdin(CE->getArg(0), C)) - return; + // Fscanf is only tainted if the input file is tainted at pre visit, so + // check for that first. + if (State->get() == PrevisitNone) + return 0; + + // Reset the taint state. + State = State->set(PrevisitNone); // All arguments except for the first two should get taint. for (unsigned int i = 2; i < CE->getNumArgs(); ++i) { @@ -159,13 +257,12 @@ void GenericTaintChecker::processFscanf(const CallExpr *CE, if (Sym) State = State->addTaint(Sym); } - C.addTransition(State); + return State; } -void GenericTaintChecker::processRetTaint(const CallExpr *CE, - CheckerContext &C) const { - const ProgramState *NewState = C.getState()->addTaint(CE); - C.addTransition(NewState); +const ProgramState *GenericTaintChecker::postRetTaint(const CallExpr *CE, + CheckerContext &C) const { + return C.getState()->addTaint(CE); } bool GenericTaintChecker::isStdin(const Expr *E, diff --git a/test/Analysis/taint-tester.c b/test/Analysis/taint-tester.c index 8c964e4ae2..85a59c50ae 100644 --- a/test/Analysis/taint-tester.c +++ b/test/Analysis/taint-tester.c @@ -150,3 +150,19 @@ void stdinTest3() { fscanf(*ppp, "%d", &iii); int jjj = iii;// expected-warning + {{tainted}} } + +// Test propagation functions - the ones that propagate taint from arguments to +// return value, ptr arguments. + +int atoi(const char *nptr); +// TODO: Add support for atol and atoll. +long atol(const char *nptr); +long long atoll(const char *nptr); + +void atoiTest() { + char s[80]; + scanf("%s", s); + int d = atoi(s); // expected-warning + {{tainted}} + int td = d; // expected-warning + {{tainted}} +} + -- 2.40.0