[cfe-commits] r147714 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp lib/StaticAnalyzer/Core/ProgramState.cpp test/Analysis/taint-generic.c
Anna Zaks
ganna at apple.com
Fri Jan 6 18:33:10 PST 2012
Author: zaks
Date: Fri Jan 6 20:33:10 2012
New Revision: 147714
URL: http://llvm.org/viewvc/llvm-project?rev=147714&view=rev
Log:
[analyzer] Add basic format string vulnerability checking.
We already have a more conservative check in the compiler (if the
format string is not a literal, we warn). Still adding it here for
completeness and since this check is stronger - only triggered if the
format string is tainted.
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
cfe/trunk/test/Analysis/taint-generic.c
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=147714&r1=147713&r2=147714&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Fri Jan 6 20:33:10 2012
@@ -308,7 +308,7 @@
bool isTainted(const Stmt *S, const LocationContext *LCtx,
TaintTagType Kind = TaintTagGeneric) const;
bool isTainted(SVal V, TaintTagType Kind = TaintTagGeneric) const;
- bool isTainted(const SymExpr* Sym, TaintTagType Kind = TaintTagGeneric) const;
+ bool isTainted(SymbolRef Sym, TaintTagType Kind = TaintTagGeneric) const;
bool isTainted(const MemRegion *Reg, TaintTagType Kind=TaintTagGeneric) const;
//==---------------------------------------------------------------------==//
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=147714&r1=147713&r2=147714&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Fri Jan 6 20:33:10 2012
@@ -43,6 +43,15 @@
mutable llvm::OwningPtr<BugType> BT;
void initBugType() const;
+ /// Add/propagate taint on a post visit.
+ void taintPost(const CallExpr *CE, CheckerContext &C) const;
+ /// Add/propagate taint on a pre visit.
+ void taintPre(const CallExpr *CE, CheckerContext &C) const;
+
+ /// Catch taint related bugs. Check if tainted data is passed to a system
+ /// call etc.
+ bool checkPre(const CallExpr *CE, CheckerContext &C) const;
+
/// Given a pointer argument, get the symbol of the value it contains
/// (points to).
SymbolRef getPointedToSymbol(CheckerContext &C,
@@ -66,6 +75,10 @@
/// and thus, is tainted.
bool isStdin(const Expr *E, CheckerContext &C) const;
+ /// Check for CWE-134: Uncontrolled Format String.
+ bool checkUncontrolledFormatString(const CallExpr *CE,
+ CheckerContext &C) const;
+
public:
static void *getTag() { static int Tag; return &Tag; }
@@ -94,13 +107,26 @@
inline void GenericTaintChecker::initBugType() const {
if (!BT)
- BT.reset(new BugType("Tainted data checking", "General"));
+ BT.reset(new BugType("Taint Analysis", "General"));
}
void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
CheckerContext &C) const {
- const ProgramState *State = C.getState();
+ // Check for errors first.
+ if (checkPre(CE, C))
+ return;
+
+ // Add taint second.
+ taintPre(CE, C);
+}
+
+void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
+ CheckerContext &C) const {
+ taintPost(CE, C);
+}
+void GenericTaintChecker::taintPre(const CallExpr *CE,
+ CheckerContext &C) const {
// Set the evaluation function by switching on the callee name.
StringRef Name = C.getCalleeName(CE);
FnCheck evalFunction = llvm::StringSwitch<FnCheck>(Name)
@@ -111,6 +137,7 @@
.Default(0);
// Check and evaluate the call.
+ const ProgramState *State = 0;
if (evalFunction)
State = (this->*evalFunction)(CE, C);
if (!State)
@@ -119,10 +146,8 @@
C.addTransition(State);
}
-void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
- CheckerContext &C) const {
- const ProgramState *State = C.getState();
-
+void GenericTaintChecker::taintPost(const CallExpr *CE,
+ CheckerContext &C) const {
// Define the attack surface.
// Set the evaluation function by switching on the callee name.
StringRef Name = C.getCalleeName(CE);
@@ -140,6 +165,7 @@
// If the callee isn't defined, it is not of security concern.
// Check and evaluate the call.
+ const ProgramState *State = 0;
if (evalFunction)
State = (this->*evalFunction)(CE, C);
if (!State)
@@ -150,6 +176,15 @@
C.addTransition(State);
}
+bool GenericTaintChecker::checkPre(const CallExpr *CE, CheckerContext &C) const{
+
+ if (checkUncontrolledFormatString(CE, C))
+ return true;
+
+ StringRef Name = C.getCalleeName(CE);
+ return false;
+}
+
SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C,
const Expr* Arg,
bool IssueWarning) const {
@@ -301,6 +336,57 @@
return false;
}
+static bool getPrintfFormatArgumentNum(const CallExpr *CE,
+ const CheckerContext &C,
+ unsigned int &ArgNum) {
+ // Find if the function contains a format string argument.
+ // Handles: fprintf, printf, sprintf, snprintf, vfprintf, vprintf, vsprintf,
+ // vsnprintf, syslog, custom annotated functions.
+ const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+ if (!FDecl)
+ return false;
+ for (specific_attr_iterator<FormatAttr>
+ i = FDecl->specific_attr_begin<FormatAttr>(),
+ e = FDecl->specific_attr_end<FormatAttr>(); i != e ; ++i) {
+
+ const FormatAttr *Format = *i;
+ ArgNum = Format->getFormatIdx() - 1;
+ if ((Format->getType() == "printf") && CE->getNumArgs() > ArgNum)
+ return true;
+ }
+
+ // Or if a function is named setproctitle (this is a heuristic).
+ if (C.getCalleeName(CE).find("setproctitle") != StringRef::npos) {
+ ArgNum = 0;
+ return true;
+ }
+
+ return false;
+}
+
+bool GenericTaintChecker::checkUncontrolledFormatString(const CallExpr *CE,
+ CheckerContext &C) const{
+ // Check if the function contains a format string argument.
+ unsigned int ArgNum = 0;
+ if (!getPrintfFormatArgumentNum(CE, C, ArgNum))
+ return false;
+
+ // If either the format string content or the pointer itself are tainted, warn.
+ const ProgramState *State = C.getState();
+ const Expr *Arg = CE->getArg(ArgNum);
+ if (State->isTainted(getPointedToSymbol(C, Arg, false)) ||
+ State->isTainted(Arg, C.getLocationContext()))
+ if (ExplodedNode *N = C.addTransition()) {
+ initBugType();
+ BugReport *report = new BugReport(*BT,
+ "Tainted format string (CWE-134: Uncontrolled Format String)", N);
+ report->addRange(Arg->getSourceRange());
+ C.EmitReport(report);
+ return true;
+ }
+ return false;
+}
+
void ento::registerGenericTaintChecker(CheckerManager &mgr) {
mgr.registerChecker<GenericTaintChecker>();
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp?rev=147714&r1=147713&r2=147714&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp Fri Jan 6 20:33:10 2012
@@ -640,7 +640,7 @@
return false;
}
-bool ProgramState::isTainted(const SymExpr* Sym, TaintTagType Kind) const {
+bool ProgramState::isTainted(SymbolRef Sym, TaintTagType Kind) const {
if (!Sym)
return false;
Modified: cfe/trunk/test/Analysis/taint-generic.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-generic.c?rev=147714&r1=147713&r2=147714&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/taint-generic.c (original)
+++ cfe/trunk/test/Analysis/taint-generic.c Fri Jan 6 20:33:10 2012
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=experimental.security.taint,experimental.security.ArrayBoundV2 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=experimental.security.taint,experimental.security.ArrayBoundV2 -Wno-format-security -verify %s
int scanf(const char *restrict format, ...);
int getchar(void);
@@ -46,3 +46,17 @@
int m = getchar();
Buffer[m] = 1; //expected-warning {{Out of bound memory access }}
}
+
+typedef struct _FILE FILE;
+extern FILE *stdin;
+int fscanf(FILE *restrict stream, const char *restrict format, ...);
+int sprintf(char *str, const char *format, ...);
+void setproctitle(const char *fmt, ...);
+
+void testUncontrolledFormatString() {
+ char s[80];
+ fscanf(stdin, "%s", s);
+ char buf[128];
+ sprintf(buf,s); // expected-warning {{Uncontrolled Format String}}
+ setproctitle(s, 3); // expected-warning {{Uncontrolled Format String}}
+}
More information about the cfe-commits
mailing list