From 110eaaceddcec790f7e6a5e3bf1261c9aa1e73ab Mon Sep 17 00:00:00 2001
From: dim <dim@FreeBSD.org>
Date: Mon, 2 May 2011 19:39:53 +0000
Subject: Vendor import of clang trunk r130700:
http://llvm.org/svn/llvm-project/cfe/trunk@130700
---
.../Checkers/CheckSecuritySyntaxOnly.cpp | 216 ++++++++++++++-------
1 file changed, 144 insertions(+), 72 deletions(-)
(limited to 'lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp')
diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 185520c..53810ee 100644
--- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -12,11 +12,12 @@
//===----------------------------------------------------------------------===//
#include "ClangSACheckers.h"
-#include "clang/StaticAnalyzer/Core/CheckerV2.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/AST/StmtVisitor.h"
#include "llvm/Support/raw_ostream.h"
+#include "llvm/ADT/StringSwitch.h"
using namespace clang;
using namespace ento;
@@ -33,21 +34,13 @@ static bool isArc4RandomAvailable(const ASTContext &Ctx) {
namespace {
class WalkAST : public StmtVisitor<WalkAST> {
BugReporter &BR;
- IdentifierInfo *II_gets;
- IdentifierInfo *II_getpw;
- IdentifierInfo *II_mktemp;
- enum { num_rands = 9 };
- IdentifierInfo *II_rand[num_rands];
- IdentifierInfo *II_random;
enum { num_setids = 6 };
IdentifierInfo *II_setid[num_setids];
const bool CheckRand;
public:
- WalkAST(BugReporter &br) : BR(br),
- II_gets(0), II_getpw(0), II_mktemp(0),
- II_rand(), II_random(0), II_setid(),
+ WalkAST(BugReporter &br) : BR(br), II_setid(),
CheckRand(isArc4RandomAvailable(BR.getContext())) {}
// Statement visitor methods.
@@ -59,16 +52,22 @@ public:
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_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
@@ -76,7 +75,7 @@ public:
// 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);
@@ -94,15 +93,43 @@ void WalkAST::VisitChildren(Stmt *S) {
}
void WalkAST::VisitCallExpr(CallExpr *CE) {
- if (const FunctionDecl *FD = CE->getDirectCallee()) {
- CheckCall_gets(CE, FD);
- CheckCall_getpw(CE, FD);
- CheckCall_mktemp(CE, FD);
- if (CheckRand) {
- CheckCall_rand(CE, FD);
- CheckCall_random(CE, FD);
- }
- }
+ // Get the callee.
+ const FunctionDecl *FD = CE->getDirectCallee();
+
+ if (!FD)
+ return;
+
+ // Get the name of the callee. If it's a builtin, strip off the prefix.
+ IdentifierInfo *II = FD->getIdentifier();
+ if (!II) // if no identifier, not a simple C function
+ return;
+ llvm::StringRef Name = II->getName();
+ if (Name.startswith("__builtin_"))
+ Name = Name.substr(10);
+
+ // 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)
+ .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.
+ // Check and evaluate the call.
+ if (evalFunction)
+ (this->*evalFunction)(CE, FD);
// Recurse and check children.
VisitChildren(CE);
@@ -112,13 +139,13 @@ void WalkAST::VisitCompoundStmt(CompoundStmt *S) {
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);
@@ -131,7 +158,7 @@ void WalkAST::VisitForStmt(ForStmt *FS) {
//===----------------------------------------------------------------------===//
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)) {
@@ -139,10 +166,10 @@ GetIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
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;
@@ -155,7 +182,7 @@ GetIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
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;
}
@@ -164,7 +191,7 @@ GetIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
/// 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();
@@ -211,7 +238,7 @@ void WalkAST::CheckLoopConditionForFloat(const ForStmt *FS) {
return;
// Does either variable appear in increment?
- const DeclRefExpr *drInc = GetIncrementedVar(increment, vdLHS, vdRHS);
+ const DeclRefExpr *drInc = getIncrementedVar(increment, vdLHS, vdRHS);
if (!drInc)
return;
@@ -243,10 +270,7 @@ void WalkAST::CheckLoopConditionForFloat(const ForStmt *FS) {
// CWE-242: Use of Inherently Dangerous Function
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
- if (FD->getIdentifier() != GetIdentifier(II_gets, "gets"))
- return;
-
+void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
const FunctionProtoType *FPT
= dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
if (!FPT)
@@ -278,10 +302,7 @@ void WalkAST::CheckCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
// CWE-477: Use of Obsolete Functions
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
- if (FD->getIdentifier() != GetIdentifier(II_getpw, "getpw"))
- return;
-
+void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
const FunctionProtoType *FPT
= dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
if (!FPT)
@@ -317,16 +338,13 @@ void WalkAST::CheckCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
// CWE-377: Insecure Temporary File
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
- if (FD->getIdentifier() != GetIdentifier(II_mktemp, "mktemp"))
- return;
-
+void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
const FunctionProtoType *FPT
= dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
if(!FPT)
return;
- // Verify that the funcion takes a single argument.
+ // Verify that the function takes a single argument.
if (FPT->getNumArgs() != 1)
return;
@@ -349,32 +367,86 @@ void WalkAST::CheckCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
}
//===----------------------------------------------------------------------===//
-// Check: Linear congruent random number generators should not be used
-// Originally: <rdar://problem/63371000>
-// CWE-338: Use of cryptographically weak prng
+// Check: Any use of 'strcpy' is insecure.
+//
+// CWE-119: Improper Restriction of Operations within
+// the Bounds of a Memory Buffer
//===----------------------------------------------------------------------===//
+void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!checkCall_strCommon(CE, FD))
+ return;
-void WalkAST::CheckCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
- if (II_rand[0] == NULL) {
- // This check applies to these functions
- static const char * const identifiers[num_rands] = {
- "drand48", "erand48", "jrand48", "lrand48", "mrand48", "nrand48",
- "lcong48",
- "rand", "rand_r"
- };
+ // 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);
+}
- for (size_t i = 0; i < num_rands; i++)
- II_rand[i] = &BR.getContext().Idents.get(identifiers[i]);
+//===----------------------------------------------------------------------===//
+// 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 false;
+
+ // Verify the function takes two arguments, three in the _chk version.
+ int numArgs = FPT->getNumArgs();
+ if (numArgs != 2 && numArgs != 3)
+ return false;
+
+ // Verify the type for both arguments.
+ for (int i = 0; i < 2; i++) {
+ // Verify that the arguments are pointers.
+ const PointerType *PT = dyn_cast<PointerType>(FPT->getArgType(i));
+ if (!PT)
+ return false;
+
+ // Verify that the argument is a 'char*'.
+ if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().CharTy)
+ return false;
}
- const IdentifierInfo *id = FD->getIdentifier();
- size_t identifierid;
+ return true;
+}
- for (identifierid = 0; identifierid < num_rands; identifierid++)
- if (id == II_rand[identifierid])
- break;
+//===----------------------------------------------------------------------===//
+// Check: Linear congruent random number generators should not be used
+// Originally: <rdar://problem/63371000>
+// CWE-338: Use of cryptographically weak prng
+//===----------------------------------------------------------------------===//
- if (identifierid >= num_rands)
+void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!CheckRand)
return;
const FunctionProtoType *FTP
@@ -415,8 +487,8 @@ void WalkAST::CheckCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
// Originally: <rdar://problem/63371000>
//===----------------------------------------------------------------------===//
-void WalkAST::CheckCall_random(const CallExpr *CE, const FunctionDecl *FD) {
- if (FD->getIdentifier() != GetIdentifier(II_random, "random"))
+void WalkAST::checkCall_random(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!CheckRand)
return;
const FunctionProtoType *FTP
@@ -442,7 +514,7 @@ void WalkAST::CheckCall_random(const CallExpr *CE, const FunctionDecl *FD) {
// Originally: <rdar://problem/6337132>
//===----------------------------------------------------------------------===//
-void WalkAST::CheckUncheckedReturnValue(CallExpr *CE) {
+void WalkAST::checkUncheckedReturnValue(CallExpr *CE) {
const FunctionDecl *FD = CE->getDirectCallee();
if (!FD)
return;
@@ -502,7 +574,7 @@ void WalkAST::CheckUncheckedReturnValue(CallExpr *CE) {
//===----------------------------------------------------------------------===//
namespace {
-class SecuritySyntaxChecker : public CheckerV2<check::ASTCodeBody> {
+class SecuritySyntaxChecker : public Checker<check::ASTCodeBody> {
public:
void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
BugReporter &BR) const {
--
cgit v1.1