Turn 'SecuritySyntaxChecker' into a "meta" security checker for insecure APIs. Now

multiple checks are exposed as separate checkers, but CheckerManager only creates
one Checker object.

llvm-svn: 148525

clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

@@ -34,6 +34,23 @@ static bool isArc4RandomAvailable(const ASTContext &Ctx) {
 }
 
 namespace {
+struct DefaultBool {
+  bool val;
+  DefaultBool() : val(false) {}
+  operator bool() const { return val; }
+  DefaultBool &operator=(bool b) { val = b; return *this; }
+};
+  
+struct ChecksFilter {
+  DefaultBool check_gets;
+  DefaultBool check_getpw;
+  DefaultBool check_mktemp;
+  DefaultBool check_strcpy;
+  DefaultBool check_rand;
+  DefaultBool check_vfork;
+  DefaultBool check_FloatLoopCounter;
+};
+  
 class WalkAST : public StmtVisitor<WalkAST> {
   BugReporter &BR;
   AnalysisDeclContext* AC;
@@ -41,11 +58,14 @@ class WalkAST : public StmtVisitor<WalkAST> {
   IdentifierInfo *II_setid[num_setids];
 
   const bool CheckRand;
+  const ChecksFilter &filter;
 
 public:
-  WalkAST(BugReporter &br, AnalysisDeclContext* ac)
+  WalkAST(BugReporter &br, AnalysisDeclContext* ac,
+          const ChecksFilter &f)
   : BR(br), AC(ac), II_setid(),
-    CheckRand(isArc4RandomAvailable(BR.getContext())) {}
+    CheckRand(isArc4RandomAvailable(BR.getContext())),
+    filter(f) {}
 
   // Statement visitor methods.
   void VisitCallExpr(CallExpr *CE);
@@ -186,6 +206,9 @@ getIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
 ///  CERT: FLP30-C, FLP30-CPP.
 ///
 void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) {
+  if (!filter.check_FloatLoopCounter)
+    return;
+
   // Does the loop have a condition?
   const Expr *condition = FS->getCond();
 
@@ -268,6 +291,9 @@ void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) {
 //===----------------------------------------------------------------------===//
 
 void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_gets)
+    return;
+  
   const FunctionProtoType *FPT
     = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
   if (!FPT)
@@ -302,6 +328,9 @@ void WalkAST::checkCall_gets(const CallExpr *CE, const FunctionDecl *FD) {
 //===----------------------------------------------------------------------===//
 
 void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_getpw)
+    return;
+
   const FunctionProtoType *FPT
     = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
   if (!FPT)
@@ -340,6 +369,9 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
 //===----------------------------------------------------------------------===//
 
 void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_mktemp)
+    return;
+
   const FunctionProtoType *FPT
     = dyn_cast<FunctionProtoType>(FD->getType().IgnoreParens());
   if(!FPT)
@@ -376,6 +408,9 @@ void WalkAST::checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD) {
 // the Bounds of a Memory Buffer 
 //===----------------------------------------------------------------------===//
 void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_strcpy)
+    return;
+  
   if (!checkCall_strCommon(CE, FD))
     return;
 
@@ -400,6 +435,9 @@ void WalkAST::checkCall_strcpy(const CallExpr *CE, const FunctionDecl *FD) {
 // the Bounds of a Memory Buffer 
 //===----------------------------------------------------------------------===//
 void WalkAST::checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_strcpy)
+    return;
+
   if (!checkCall_strCommon(CE, FD))
     return;
 
@@ -453,7 +491,7 @@ bool WalkAST::checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD) {
 //===----------------------------------------------------------------------===//
 
 void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
-  if (!CheckRand)
+  if (!filter.check_rand || !CheckRand)
     return;
 
   const FunctionProtoType *FTP
@@ -526,6 +564,9 @@ void WalkAST::checkCall_random(const CallExpr *CE, const FunctionDecl *FD) {
 //===----------------------------------------------------------------------===//
 
 void WalkAST::checkCall_vfork(const CallExpr *CE, const FunctionDecl *FD) {
+  if (!filter.check_vfork)
+    return;
+
   // All calls to vfork() are insecure, issue a warning.
   SourceRange R = CE->getCallee()->getSourceRange();
   PathDiagnosticLocation CELoc =
@@ -609,14 +650,26 @@ void WalkAST::checkUncheckedReturnValue(CallExpr *CE) {
 namespace {
 class SecuritySyntaxChecker : public Checker<check::ASTCodeBody> {
 public:
+  ChecksFilter filter;
+  
   void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
                         BugReporter &BR) const {
-    WalkAST walker(BR, mgr.getAnalysisDeclContext(D));
+    WalkAST walker(BR, mgr.getAnalysisDeclContext(D), filter);
     walker.Visit(D->getBody());
   }
 };
 }
 
-void ento::registerSecuritySyntaxChecker(CheckerManager &mgr) {
-  mgr.registerChecker<SecuritySyntaxChecker>();
+#define REGISTER_CHECKER(name) \
+void ento::register##name(CheckerManager &mgr) {\
+  mgr.registerChecker<SecuritySyntaxChecker>()->filter.check_##name = true;\
 }
+
+REGISTER_CHECKER(gets)
+REGISTER_CHECKER(getpw)
+REGISTER_CHECKER(mktemp)
+REGISTER_CHECKER(strcpy)
+REGISTER_CHECKER(rand)
+REGISTER_CHECKER(vfork)
+REGISTER_CHECKER(FloatLoopCounter)
+

clang/lib/StaticAnalyzer/Checkers/Checkers.td

@@ -27,6 +27,7 @@ def DeadCode : Package<"deadcode">;
 def DeadCodeExperimental : Package<"deadcode">, InPackage<Experimental>, Hidden;
 
 def Security : Package <"security">;
+def InsecureAPI : Package<"insecureAPI">, InPackage<Security>;
 def SecurityExperimental : Package<"security">, InPackage<Experimental>, Hidden;
 def Taint : Package<"taint">, InPackage<SecurityExperimental>, Hidden;  
 
@@ -198,11 +199,33 @@ def UnreachableCodeChecker : Checker<"UnreachableCode">,
 // Security checkers.
 //===----------------------------------------------------------------------===//
 
-let ParentPackage = SecurityExperimental in {
-
-def SecuritySyntaxChecker : Checker<"SecuritySyntactic">,
-  HelpText<"Perform quick security API checks that require no data flow">,
+let ParentPackage = InsecureAPI in {
+  def gets : Checker<"gets">,
+    HelpText<"Warn on uses of the 'gets' function">,
     DescFile<"CheckSecuritySyntaxOnly.cpp">;
+  def getpw : Checker<"getpw">,
+    HelpText<"Warn on uses of the 'getpw' function">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
+  def mktemp : Checker<"mktemp">,
+    HelpText<"Warn on uses of the 'mktemp' function">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
+  def rand : Checker<"rand">,
+    HelpText<"Warn on uses of the 'rand', 'random', and related functions">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
+  def strcpy : Checker<"strcpy">,
+    HelpText<"Warn on uses of the 'strcpy' and 'strcat' functions">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
+  def vfork : Checker<"vfork">,
+    HelpText<"Warn on uses of the 'vfork' function">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
+}
+let ParentPackage = Security in {
+  def FloatLoopCounter : Checker<"FloatLoopCounter">,
+    HelpText<"Warn on using a floating point value as a loop counter (CERT: FLP30-C, FLP30-CPP)">,
+    DescFile<"CheckSecuritySyntaxOnly.cpp">;
+}
+
+let ParentPackage = SecurityExperimental in {
 
 def ArrayBoundChecker : Checker<"ArrayBound">,
   HelpText<"Warn about buffer overflows (older checker)">,

clang/test/Analysis/security-syntax-checks-no-emit.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i686-pc-linux-gnu -analyze -analyzer-checker=experimental.security.SecuritySyntactic %s -verify
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -analyze -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify
 
 // This file complements 'security-syntax-checks.m', but tests that we omit
 // specific checks on platforms where they don't make sense.

clang/test/Analysis/security-syntax-checks.m

@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=experimental.security.SecuritySyntactic %s -verify
-// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DUSE_BUILTINS -analyzer-checker=experimental.security.SecuritySyntactic %s -verify
-// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DVARIANT -analyzer-checker=experimental.security.SecuritySyntactic %s -verify
-// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=experimental.security.SecuritySyntactic %s -verify
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DUSE_BUILTINS -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DVARIANT -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -DUSE_BUILTINS -DVARIANT -analyzer-checker=security.insecureAPI,security.FloatLoopCounter %s -verify
 
 #ifdef USE_BUILTINS
 # define BUILTIN(f) __builtin_ ## f