[cfe-commits] r148371 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp test/Analysis/taint-generic.c
Anna Zaks
ganna at apple.com
Tue Jan 17 18:45:11 PST 2012
Author: zaks
Date: Tue Jan 17 20:45:11 2012
New Revision: 148371
URL: http://llvm.org/viewvc/llvm-project?rev=148371&view=rev
Log:
[analyzer] Taint: warn when tainted data is used to specify a buffer
size (Ex: in malloc, memcpy, strncpy..)
(Maybe some of this could migrate to the CString checker. One issue
with that is that we might want to separate security issues from
regular API misuse.)
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
cfe/trunk/test/Analysis/taint-generic.c
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=148371&r1=148370&r2=148371&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Tue Jan 17 20:45:11 2012
@@ -99,6 +99,12 @@
bool checkSystemCall(const CallExpr *CE, StringRef Name,
CheckerContext &C) const;
+ /// Check if tainted data is used as a buffer size ins strn.. functions,
+ /// and allocators.
+ static const char MsgTaintedBufferSize[];
+ bool checkTaintedBufferSize(const CallExpr *CE, const FunctionDecl *FDecl,
+ CheckerContext &C) const;
+
/// Generate a report if the expression is tainted or points to tainted data.
bool generateReportIfTainted(const Expr *E, const char Msg[],
CheckerContext &C) const;
@@ -176,7 +182,13 @@
const char GenericTaintChecker::MsgSanitizeSystemArgs[] =
"Tainted data passed to a system call "
"(CERT/STR02-C. Sanitize data passed to complex subsystems)";
-}
+
+const char GenericTaintChecker::MsgTaintedBufferSize[] =
+ "Tainted data is used to specify the buffer size "
+ "(CERT/STR31-C. Guarantee that storage for strings has sufficient space for "
+ "character data and the null terminator)";
+
+} // end of anonymous namespace
/// A set which is used to pass information from call pre-visit instruction
/// to the call post-visit. The values are unsigned integers, which are either
@@ -242,8 +254,8 @@
else if (C.isCLibraryFunction(FDecl, "strdup") ||
C.isCLibraryFunction(FDecl, "strdupa"))
return TaintPropagationRule(0, ReturnValueIndex);
- else if (C.isCLibraryFunction(FDecl, "strndupa"))
- return TaintPropagationRule(0, 1, ReturnValueIndex);
+ else if (C.isCLibraryFunction(FDecl, "wcsdup"))
+ return TaintPropagationRule(0, ReturnValueIndex);
}
// Skipping the following functions, since they might be used for cleansing
@@ -371,13 +383,17 @@
if (checkUncontrolledFormatString(CE, C))
return true;
- StringRef Name = C.getCalleeName(CE);
+ const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+ StringRef Name = C.getCalleeName(FDecl);
if (Name.empty())
return false;
if (checkSystemCall(CE, Name, C))
return true;
+ if (checkTaintedBufferSize(CE, FDecl, C))
+ return true;
+
return false;
}
@@ -639,6 +655,48 @@
return false;
}
+// TODO: Should this check be a part of the CString checker?
+// If yes, should taint be a global setting?
+bool GenericTaintChecker::checkTaintedBufferSize(const CallExpr *CE,
+ const FunctionDecl *FDecl,
+ CheckerContext &C) const {
+ // If the function has a buffer size argument, set ArgNum.
+ unsigned ArgNum = InvalidArgIndex;
+ unsigned BId = 0;
+ if ( (BId = FDecl->getMemoryFunctionKind()) )
+ switch(BId) {
+ case Builtin::BImemcpy:
+ case Builtin::BImemmove:
+ case Builtin::BIstrncpy:
+ ArgNum = 2;
+ break;
+ case Builtin::BIstrndup:
+ ArgNum = 1;
+ break;
+ default:
+ break;
+ };
+
+ if (ArgNum == InvalidArgIndex) {
+ if (C.isCLibraryFunction(FDecl, "malloc") ||
+ C.isCLibraryFunction(FDecl, "calloc") ||
+ C.isCLibraryFunction(FDecl, "alloca"))
+ ArgNum = 0;
+ else if (C.isCLibraryFunction(FDecl, "memccpy"))
+ ArgNum = 3;
+ else if (C.isCLibraryFunction(FDecl, "realloc"))
+ ArgNum = 1;
+ else if (C.isCLibraryFunction(FDecl, "bcopy"))
+ ArgNum = 2;
+ }
+
+ if (ArgNum != InvalidArgIndex &&
+ generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C))
+ return true;
+
+ return false;
+}
+
void ento::registerGenericTaintChecker(CheckerManager &mgr) {
mgr.registerChecker<GenericTaintChecker>();
}
Modified: cfe/trunk/test/Analysis/taint-generic.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-generic.c?rev=148371&r1=148370&r2=148371&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/taint-generic.c (original)
+++ cfe/trunk/test/Analysis/taint-generic.c Tue Jan 17 20:45:11 2012
@@ -23,6 +23,11 @@
char *stpcpy(char *restrict s1, const char *restrict s2);
char *strncpy( char * destination, const char * source, size_t num );
char *strndup(const char *s, size_t n);
+char *strncat(char *restrict s1, const char *restrict s2, size_t n);
+
+void *malloc(size_t);
+void *calloc(size_t nmemb, size_t size);
+void bcopy(void *s1, void *s2, size_t n);
#define BUFSIZE 10
@@ -112,6 +117,7 @@
sprintf(buffer, "/bin/mail %s < /tmp/email", addr);
system(buffer); // expected-warning {{Tainted data passed to a system call}}
}
+
void testTaintSystemCall2() {
// Test that snpintf transfers taint.
char buffern[156];
@@ -120,6 +126,7 @@
__builtin_snprintf(buffern, 10, "/bin/mail %s < /tmp/email", addr);
system(buffern); // expected-warning {{Tainted data passed to a system call}}
}
+
void testTaintSystemCall3() {
char buffern2[156];
int numt;
@@ -128,3 +135,18 @@
__builtin_snprintf(buffern2, numt, "/bin/mail %s < /tmp/email", "abcd");
system(buffern2); // expected-warning {{Tainted data passed to a system call}}
}
+
+void testTaintedBufferSize() {
+ size_t ts;
+ scanf("%zd", &ts);
+
+ int *buf1 = (int*)malloc(ts*sizeof(int)); // expected-warning {{Tainted data is used to specify the buffer size}}
+ char *dst = (char*)calloc(ts, sizeof(char)); //expected-warning {{Tainted data is used to specify the buffer size}}
+ bcopy(buf1, dst, ts); // expected-warning {{Tainted data is used to specify the buffer size}}
+ __builtin_memcpy(dst, buf1, (ts + 4)*sizeof(char)); // expected-warning {{Tainted data is used to specify the buffer size}}
+
+ // If both buffers are trusted, do not issue a warning.
+ char *dst2 = (char*)malloc(ts*sizeof(char)); // expected-warning {{Tainted data is used to specify the buffer size}}
+ strncat(dst2, dst, ts); // no-warning
+
+}
More information about the cfe-commits
mailing list