[clang][analyzer] Add new option to specify functions SecuritySyntaxChecker warns about

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -839,79 +839,89 @@ def PaddingChecker : Checker<"Padding">,
 
 let ParentPackage = InsecureAPI in {
 
-def SecuritySyntaxChecker : Checker<"SecuritySyntaxChecker">,
-  HelpText<"Base of various security function related checkers">,
-  Documentation<NotDocumented>,
-  Hidden;
-
-def bcmp : Checker<"bcmp">,
-  HelpText<"Warn on uses of the 'bcmp' function">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
-
-def bcopy : Checker<"bcopy">,
-  HelpText<"Warn on uses of the 'bcopy' function">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
-
-def bzero : Checker<"bzero">,
-  HelpText<"Warn on uses of the 'bzero' function">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
-
-def gets : Checker<"gets">,
-  HelpText<"Warn on uses of the 'gets' function">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
-
-def getpw : Checker<"getpw">,
-  HelpText<"Warn on uses of the 'getpw' function">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
-
-def mktemp : Checker<"mktemp">,
-  HelpText<"Warn on uses of the 'mktemp' function">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
-
-def mkstemp : Checker<"mkstemp">,
-  HelpText<"Warn when 'mkstemp' is passed fewer than 6 X's in the format "
-           "string">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
+  def SecuritySyntaxChecker
+      : Checker<"SecuritySyntaxChecker">,
+        HelpText<"Base of various security function related checkers">,
+        CheckerOptions<[CmdLineOption<
+            String, "Warn",
+            "List of space-separated function name to be warned about. "
+            "Defaults to an empty list.",
+            "", InAlpha>]>,
+        Documentation<NotDocumented>,
+        Hidden;
+
+  def bcmp : Checker<"bcmp">,
+             HelpText<"Warn on uses of the 'bcmp' function">,
+             Dependencies<[SecuritySyntaxChecker]>,
+             Documentation<HasDocumentation>;
+
+  def bcopy : Checker<"bcopy">,
+              HelpText<"Warn on uses of the 'bcopy' function">,
+              Dependencies<[SecuritySyntaxChecker]>,
+              Documentation<HasDocumentation>;
+
+  def bzero : Checker<"bzero">,
+              HelpText<"Warn on uses of the 'bzero' function">,
+              Dependencies<[SecuritySyntaxChecker]>,
+              Documentation<HasDocumentation>;
+
+  def gets : Checker<"gets">,
+             HelpText<"Warn on uses of the 'gets' function">,
+             Dependencies<[SecuritySyntaxChecker]>,
+             Documentation<HasDocumentation>;
+
+  def getpw : Checker<"getpw">,
+              HelpText<"Warn on uses of the 'getpw' function">,
+              Dependencies<[SecuritySyntaxChecker]>,
+              Documentation<HasDocumentation>;
+
+  def mktemp : Checker<"mktemp">,
+               HelpText<"Warn on uses of the 'mktemp' function">,
+               Dependencies<[SecuritySyntaxChecker]>,
+               Documentation<HasDocumentation>;
+
+  def mkstemp
+      : Checker<"mkstemp">,
+        HelpText<"Warn when 'mkstemp' is passed fewer than 6 X's in the format "
+                 "string">,
+        Dependencies<[SecuritySyntaxChecker]>,
+        Documentation<HasDocumentation>;
 
-def rand : Checker<"rand">,
-  HelpText<"Warn on uses of the 'rand', 'random', and related functions">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
+  def rand
+      : Checker<"rand">,
+        HelpText<"Warn on uses of the 'rand', 'random', and related functions">,
+        Dependencies<[SecuritySyntaxChecker]>,
+        Documentation<HasDocumentation>;
 
-def strcpy : Checker<"strcpy">,
-  HelpText<"Warn on uses of the 'strcpy' and 'strcat' functions">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
+  def strcpy : Checker<"strcpy">,
+               HelpText<"Warn on uses of the 'strcpy' and 'strcat' functions">,
+               Dependencies<[SecuritySyntaxChecker]>,
+               Documentation<HasDocumentation>;
 
-def vfork : Checker<"vfork">,
-  HelpText<"Warn on uses of the 'vfork' function">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
+  def vfork : Checker<"vfork">,
+              HelpText<"Warn on uses of the 'vfork' function">,
+              Dependencies<[SecuritySyntaxChecker]>,
+              Documentation<HasDocumentation>;
 
-def UncheckedReturn : Checker<"UncheckedReturn">,
-  HelpText<"Warn on uses of functions whose return values must be always "
-           "checked">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
+  def UncheckedReturn
+      : Checker<"UncheckedReturn">,
+        HelpText<"Warn on uses of functions whose return values must be always "
+                 "checked">,
+        Dependencies<[SecuritySyntaxChecker]>,
+        Documentation<HasDocumentation>;
 
-def DeprecatedOrUnsafeBufferHandling :
-  Checker<"DeprecatedOrUnsafeBufferHandling">,
-  HelpText<"Warn on uses of unsecure or deprecated buffer manipulating "
-           "functions">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
+  def DeprecatedOrUnsafeBufferHandling
+      : Checker<"DeprecatedOrUnsafeBufferHandling">,
+        HelpText<"Warn on uses of unsecure or deprecated buffer manipulating "
+                 "functions">,
+        Dependencies<[SecuritySyntaxChecker]>,
+        Documentation<HasDocumentation>;
 
-def decodeValueOfObjCType : Checker<"decodeValueOfObjCType">,
-  HelpText<"Warn on uses of the '-decodeValueOfObjCType:at:' method">,
-  Dependencies<[SecuritySyntaxChecker]>,
-  Documentation<HasDocumentation>;
+  def decodeValueOfObjCType
+      : Checker<"decodeValueOfObjCType">,
+        HelpText<"Warn on uses of the '-decodeValueOfObjCType:at:' method">,
+        Dependencies<[SecuritySyntaxChecker]>,
+        Documentation<HasDocumentation>;
 
 } // end "security.insecureAPI"
 

clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

@@ -53,6 +53,7 @@ struct ChecksFilter {
   CheckerNameRef checkName_bcmp;
   CheckerNameRef checkName_bcopy;
   CheckerNameRef checkName_bzero;
+  CheckerNameRef checkName_custom;
   CheckerNameRef checkName_gets;
   CheckerNameRef checkName_getpw;
   CheckerNameRef checkName_mktemp;
@@ -74,13 +75,14 @@ class WalkAST : public StmtVisitor<WalkAST> {
 
   const bool CheckRand;
   const ChecksFilter &filter;
+  llvm::StringSet<> WarnFunctions;
 
 public:
-  WalkAST(BugReporter &br, AnalysisDeclContext* ac,
-          const ChecksFilter &f)
-  : BR(br), AC(ac), II_setid(),
-    CheckRand(isArc4RandomAvailable(BR.getContext())),
-    filter(f) {}
+  WalkAST(BugReporter &br, AnalysisDeclContext *ac, const ChecksFilter &f,
+          llvm::StringSet<> wf)
+      : BR(br), AC(ac), II_setid(),
+        CheckRand(isArc4RandomAvailable(BR.getContext())), filter(f),
+        WarnFunctions(wf) {}
 
   // Statement visitor methods.
   void VisitCallExpr(CallExpr *CE);
@@ -101,6 +103,7 @@ class WalkAST : public StmtVisitor<WalkAST> {
   void checkLoopConditionForFloat(const ForStmt *FS);
   void checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_bcopy(const CallExpr *CE, const FunctionDecl *FD);
+  void checkCall_custom(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_bzero(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_gets(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD);
@@ -175,12 +178,10 @@ void WalkAST::VisitCallExpr(CallExpr *CE) {
           .Case("rand_r", &WalkAST::checkCall_rand)
           .Case("random", &WalkAST::checkCall_random)
           .Case("vfork", &WalkAST::checkCall_vfork)
-          .Default(nullptr);
+          .Default(&WalkAST::checkCall_custom);
 
-  // If the callee isn't defined, it is not of security concern.
   // Check and evaluate the call.
-  if (evalFunction)
-    (this->*evalFunction)(CE, FD);
+  (this->*evalFunction)(CE, FD);
 
   // Recurse and check children.
   VisitChildren(CE);
@@ -542,6 +543,29 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
                      CELoc, CE->getCallee()->getSourceRange());
 }
 
+//===----------------------------------------------------------------------===//
+// Check: Any use of a function from the user-provided list.
+//===----------------------------------------------------------------------===//
+
+void WalkAST::checkCall_custom(const CallExpr *CE, const FunctionDecl *FD) {
+  IdentifierInfo *II = FD->getIdentifier();
+  if (!II) // if no identifier, not a simple C function
+    return;
+  StringRef Name = II->getName();
+  Name.consume_front("__builtin_");
+
+  if (!(this->WarnFunctions.contains(Name)))
+    return;
+
+  // Issue a warning.
+  std::string Msg = ("Call to user-defined function '" + Name + "'.").str();
+  PathDiagnosticLocation CELoc =
+      PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
+  BR.EmitBasicReport(AC->getDecl(), filter.checkName_custom,
+                     "User-provided function to be warned about", "Security",
+                     Msg, CELoc, CE->getCallee()->getSourceRange());
+}
+
 //===----------------------------------------------------------------------===//
 // Check: Any use of 'mktemp' is insecure.  It is obsoleted by mkstemp().
 // CWE-377: Insecure Temporary File
@@ -1075,17 +1099,30 @@ namespace {
 class SecuritySyntaxChecker : public Checker<check::ASTCodeBody> {
 public:
   ChecksFilter filter;
+  llvm::StringSet<> WarnFunctions;
 
   void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
                         BugReporter &BR) const {
-    WalkAST walker(BR, mgr.getAnalysisDeclContext(D), filter);
+    WalkAST walker(BR, mgr.getAnalysisDeclContext(D), filter, WarnFunctions);
     walker.Visit(D->getBody());
   }
+
+  void setWarnFunctions(StringRef Input) {
+    StringRef CurrentStr = Input;
+    while (!CurrentStr.empty()) {
+      auto SplitResult = CurrentStr.split(' ');
+      WarnFunctions.insert(SplitResult.first);
+      CurrentStr = SplitResult.second;
+    }
+  }
 };
 }
 
 void ento::registerSecuritySyntaxChecker(CheckerManager &mgr) {
-  mgr.registerChecker<SecuritySyntaxChecker>();
+  SecuritySyntaxChecker *checker = mgr.registerChecker<SecuritySyntaxChecker>();
+  StringRef WarnOption =
+      mgr.getAnalyzerOptions().getCheckerStringOption(checker, "Warn");
+  checker->setWarnFunctions(WarnOption);
 }
 
 bool ento::shouldRegisterSecuritySyntaxChecker(const CheckerManager &mgr) {

clang/test/Analysis/analyzer-config.c

@@ -121,6 +121,7 @@
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: report-in-main-source-file = false
 // CHECK-NEXT: security.cert.env.InvalidPtr:InvalidatingGetEnv = false
+// CHECK-NEXT: security.insecureAPI.SecuritySyntaxChecker:Warn = ""
 // CHECK-NEXT: serialize-stats = false
 // CHECK-NEXT: silence-checkers = ""
 // CHECK-NEXT: stable-report-filename = false