From 8f4caf5fec2de9b18f9c5fc69696d9f6cf66bcc5 Mon Sep 17 00:00:00 2001 From: Anna Zaks <ganna@apple.com> Date: Fri, 18 Nov 2011 02:26:36 +0000 Subject: [PATCH] [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 --- .../Checkers/GenericTaintChecker.cpp | 39 +++++++++++++++++-- lib/StaticAnalyzer/Core/SValBuilder.cpp | 1 - test/Analysis/taint-generic.c | 5 +++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 0fcfdf8ff59..c8e54efb673 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 db2097c16f2..778a0bf97da 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 2e3def36709..54229937d14 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}} +} -- GitLab