[cfe-commits] r128916 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp test/Analysis/security-syntax-checks.m
Lenny Maiorani
lenny at colorado.edu
Tue Apr 5 13:18:46 PDT 2011
Author: lenny
Date: Tue Apr 5 15:18:46 2011
New Revision: 128916
URL: http://llvm.org/viewvc/llvm-project?rev=128916&view=rev
Log:
Add security syntax checker for strcat() which causes the Static Analyzer to generate a warning any time the strcat() function is used with a note suggesting to use a function which provides bounded buffers. CWE-119.
Also, brings the security syntax checker more inline with coding standards.
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
cfe/trunk/test/Analysis/security-syntax-checks.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp?rev=128916&r1=128915&r2=128916&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp Tue Apr 5 15:18:46 2011
@@ -52,21 +52,22 @@
void VisitChildren(Stmt *S);
// Helpers.
- IdentifierInfo *GetIdentifier(IdentifierInfo *& II, const char *str);
+ IdentifierInfo *getIdentifier(IdentifierInfo *& II, const char *str);
+ bool checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD);
typedef void (WalkAST::*FnCheck)(const CallExpr *,
const FunctionDecl *);
// Checker-specific methods.
- void CheckLoopConditionForFloat(const ForStmt *FS);
- void CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD);
- void CheckCall_getpw(const CallExpr *CE, const FunctionDecl *FD);
- void CheckCall_mktemp(const CallExpr *CE, const FunctionDecl *FD);
- void CheckCall_strcpy(const CallExpr *CE, const FunctionDecl *FD);
- void CheckCall_strcat(const CallExpr *CE, const FunctionDecl *FD);
- void CheckCall_rand(const CallExpr *CE, const FunctionDecl *FD);
- void CheckCall_random(const CallExpr *CE, const FunctionDecl *FD);
- void CheckUncheckedReturnValue(CallExpr *CE);
+ void checkLoopConditionForFloat(const ForStmt *FS);
+ void checkCall_gets(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_rand(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_random(const CallExpr *CE, const FunctionDecl *FD);
+ void checkUncheckedReturnValue(CallExpr *CE);
};
} // end anonymous namespace
@@ -74,7 +75,7 @@
// Helper methods.
//===----------------------------------------------------------------------===//
-IdentifierInfo *WalkAST::GetIdentifier(IdentifierInfo *& II, const char *str) {
+IdentifierInfo *WalkAST::getIdentifier(IdentifierInfo *& II, const char *str) {
if (!II)
II = &BR.getContext().Idents.get(str);
@@ -108,20 +109,21 @@
// Set the evaluation function by switching on the callee name.
FnCheck evalFunction = llvm::StringSwitch<FnCheck>(Name)
- .Case("gets", &WalkAST::CheckCall_gets)
- .Case("getpw", &WalkAST::CheckCall_getpw)
- .Case("mktemp", &WalkAST::CheckCall_mktemp)
- .Cases("strcpy", "__strcpy_chk", &WalkAST::CheckCall_strcpy)
- .Case("drand48", &WalkAST::CheckCall_rand)
- .Case("erand48", &WalkAST::CheckCall_rand)
- .Case("jrand48", &WalkAST::CheckCall_rand)
- .Case("lrand48", &WalkAST::CheckCall_rand)
- .Case("mrand48", &WalkAST::CheckCall_rand)
- .Case("nrand48", &WalkAST::CheckCall_rand)
- .Case("lcong48", &WalkAST::CheckCall_rand)
- .Case("rand", &WalkAST::CheckCall_rand)
- .Case("rand_r", &WalkAST::CheckCall_rand)
- .Case("random", &WalkAST::CheckCall_random)
+ .Case("gets", &WalkAST::checkCall_gets)
+ .Case("getpw", &WalkAST::checkCall_getpw)
+ .Case("mktemp", &WalkAST::checkCall_mktemp)
+ .Cases("strcpy", "__strcpy_chk", &WalkAST::checkCall_strcpy)
+ .Cases("strcat", "__strcat_chk", &WalkAST::checkCall_strcat)
+ .Case("drand48", &WalkAST::checkCall_rand)
+ .Case("erand48", &WalkAST::checkCall_rand)
+ .Case("jrand48", &WalkAST::checkCall_rand)
+ .Case("lrand48", &WalkAST::checkCall_rand)
+ .Case("mrand48", &WalkAST::checkCall_rand)
+ .Case("nrand48", &WalkAST::checkCall_rand)
+ .Case("lcong48", &WalkAST::checkCall_rand)
+ .Case("rand", &WalkAST::checkCall_rand)
+ .Case("rand_r", &WalkAST::checkCall_rand)
+ .Case("random", &WalkAST::checkCall_random)
.Default(NULL);
// If the callee isn't defined, it is not of security concern.
@@ -137,13 +139,13 @@
for (Stmt::child_iterator I = S->child_begin(), E = S->child_end(); I!=E; ++I)
if (Stmt *child = *I) {
if (CallExpr *CE = dyn_cast<CallExpr>(child))
- CheckUncheckedReturnValue(CE);
+ checkUncheckedReturnValue(CE);
Visit(child);
}
}
void WalkAST::VisitForStmt(ForStmt *FS) {
- CheckLoopConditionForFloat(FS);
+ checkLoopConditionForFloat(FS);
// Recurse and check children.
VisitChildren(FS);
@@ -156,7 +158,7 @@
//===----------------------------------------------------------------------===//
static const DeclRefExpr*
-GetIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
+getIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
expr = expr->IgnoreParenCasts();
if (const BinaryOperator *B = dyn_cast<BinaryOperator>(expr)) {
@@ -164,10 +166,10 @@
B->getOpcode() == BO_Comma))
return NULL;
- if (const DeclRefExpr *lhs = GetIncrementedVar(B->getLHS(), x, y))
+ if (const DeclRefExpr *lhs = getIncrementedVar(B->getLHS(), x, y))
return lhs;
- if (const DeclRefExpr *rhs = GetIncrementedVar(B->getRHS(), x, y))
+ if (const DeclRefExpr *rhs = getIncrementedVar(B->getRHS(), x, y))
return rhs;
return NULL;
@@ -180,7 +182,7 @@
if (const UnaryOperator *U = dyn_cast<UnaryOperator>(expr))
return U->isIncrementDecrementOp()
- ? GetIncrementedVar(U->getSubExpr(), x, y) : NULL;
+ ? getIncrementedVar(U->getSubExpr(), x, y) : NULL;
return NULL;
}
@@ -189,7 +191,7 @@
/// use a floating point variable as a loop counter.
/// CERT: FLP30-C, FLP30-CPP.
///
-void WalkAST::CheckLoopConditionForFloat(const ForStmt *FS) {
+void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) {
// Does the loop have a condition?
const Expr *condition = FS->getCond();
@@ -236,7 +238,7 @@
return;
// Does either variable appear in increment?
- const DeclRefExpr *drInc = GetIncrementedVar(increment, vdLHS, vdRHS);
+ const DeclRefExpr *drInc = getIncrementedVar(increment, vdLHS, vdRHS);
if (!drInc)
return;
@@ -268,7 +270,7 @@
// CWE-242: Use of Inherently Dangerous Function
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
+void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
const FunctionProtoType *FPT
= dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
if (!FPT)
@@ -300,7 +302,7 @@
// CWE-477: Use of Obsolete Functions
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
+void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
const FunctionProtoType *FPT
= dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
if (!FPT)
@@ -336,7 +338,7 @@
// CWE-377: Insecure Temporary File
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
+void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
const FunctionProtoType *FPT
= dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
if(!FPT)
@@ -370,39 +372,71 @@
// CWE-119: Improper Restriction of Operations within
// the Bounds of a Memory Buffer
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
+void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!checkCall_strCommon(CE, FD))
+ return;
+
+ // Issue a warning.
+ SourceRange R = CE->getCallee()->getSourceRange();
+ BR.EmitBasicReport("Potential insecure memory buffer bounds restriction in "
+ "call 'strcpy'",
+ "Security",
+ "Call to function 'strcpy' is insecure as it does not "
+ "provide bounding of the memory buffer. Replace "
+ "unbounded copy functions with analogous functions that "
+ "support length arguments such as 'strncpy'. CWE-119.",
+ CE->getLocStart(), &R, 1);
+}
+
+//===----------------------------------------------------------------------===//
+// Check: Any use of 'strcat' is insecure.
+//
+// CWE-119: Improper Restriction of Operations within
+// the Bounds of a Memory Buffer
+//===----------------------------------------------------------------------===//
+void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!checkCall_strCommon(CE, FD))
+ return;
+
+ // Issue a warning.
+ SourceRange R = CE->getCallee()->getSourceRange();
+ BR.EmitBasicReport("Potential insecure memory buffer bounds restriction in "
+ "call 'strcat'",
+ "Security",
+ "Call to function 'strcat' is insecure as it does not "
+ "provide bounding of the memory buffer. Replace "
+ "unbounded copy functions with analogous functions that "
+ "support length arguments such as 'strncat'. CWE-119.",
+ CE->getLocStart(), &R, 1);
+}
+
+//===----------------------------------------------------------------------===//
+// Common check for str* functions with no bounds parameters.
+//===----------------------------------------------------------------------===//
+bool WalkAST::checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD) {
const FunctionProtoType *FPT
= dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
if (!FPT)
- return;
+ return false;
- // Verify the function takes two arguments
+ // Verify the function takes two arguments, three in the _chk version.
int numArgs = FPT->getNumArgs();
if (numArgs != 2 && numArgs != 3)
- return;
+ return false;
- // Verify the type for both arguments
+ // Verify the type for both arguments.
for (int i = 0; i < 2; i++) {
- // Verify that the arguments are pointers
+ // Verify that the arguments are pointers.
const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(i));
if (!PT)
- return;
+ return false;
// Verify that the argument is a 'char*'.
if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().CharTy)
- return;
+ return false;
}
- // Issue a warning
- SourceRange R = CE->getCallee()->getSourceRange();
- BR.EmitBasicReport("Potential insecure memory buffer bounds restriction in "
- "call 'strcpy'",
- "Security",
- "Call to function 'strcpy' is insecure as it does not "
- "provide bounding of the memory buffer. Replace "
- "unbounded copy functions with analogous functions that "
- "support length arguments such as 'strncpy'. CWE-119.",
- CE->getLocStart(), &R, 1);
+ return true;
}
//===----------------------------------------------------------------------===//
@@ -411,7 +445,7 @@
// CWE-338: Use of cryptographically weak prng
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
+void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
if (!CheckRand)
return;
@@ -453,7 +487,7 @@
// Originally: <rdar://problem/63371000>
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_random(const CallExpr *CE, const FunctionDecl *FD) {
+void WalkAST::checkCall_random(const CallExpr *CE, const FunctionDecl *FD) {
if (!CheckRand)
return;
@@ -480,7 +514,7 @@
// Originally: <rdar://problem/6337132>
//===----------------------------------------------------------------------===//
-void WalkAST::CheckUncheckedReturnValue(CallExpr *CE) {
+void WalkAST::checkUncheckedReturnValue(CallExpr *CE) {
const FunctionDecl *FD = CE->getDirectCallee();
if (!FD)
return;
Modified: cfe/trunk/test/Analysis/security-syntax-checks.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/security-syntax-checks.m?rev=128916&r1=128915&r2=128916&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/security-syntax-checks.m (original)
+++ cfe/trunk/test/Analysis/security-syntax-checks.m Tue Apr 5 15:18:46 2011
@@ -140,3 +140,27 @@
strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strncpy'. CWE-119.}}
}
+
+//===----------------------------------------------------------------------===
+// strcat()
+//===----------------------------------------------------------------------===
+#ifdef VARIANT
+
+#define __strcat_chk BUILTIN(__strcat_chk)
+char *__strcat_chk(char *restrict s1, const char *restrict s2, size_t destlen);
+
+#define strcat(a,b) __strcat_chk(a,b,(size_t)-1)
+
+#else /* VARIANT */
+
+#define strcat BUILTIN(strcat)
+char *strcat(char *restrict s1, const char *restrict s2);
+
+#endif /* VARIANT */
+
+void test_strcat() {
+ char x[4];
+ char *y;
+
+ strcat(x, y); //expected-warning{{Call to function 'strcat' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strncat'. CWE-119.}}
+}
More information about the cfe-commits
mailing list