diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index ffae3b9310979..da8985ce44c0c 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/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, - Hidden; - -def bcmp : Checker<"bcmp">, - HelpText<"Warn on uses of the 'bcmp' function">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; - -def bcopy : Checker<"bcopy">, - HelpText<"Warn on uses of the 'bcopy' function">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; - -def bzero : Checker<"bzero">, - HelpText<"Warn on uses of the 'bzero' function">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; - -def gets : Checker<"gets">, - HelpText<"Warn on uses of the 'gets' function">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; - -def getpw : Checker<"getpw">, - HelpText<"Warn on uses of the 'getpw' function">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; - -def mktemp : Checker<"mktemp">, - HelpText<"Warn on uses of the 'mktemp' function">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; - -def mkstemp : Checker<"mkstemp">, - HelpText<"Warn when 'mkstemp' is passed fewer than 6 X's in the format " - "string">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; + 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, + Hidden; + + def bcmp : Checker<"bcmp">, + HelpText<"Warn on uses of the 'bcmp' function">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; + + def bcopy : Checker<"bcopy">, + HelpText<"Warn on uses of the 'bcopy' function">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; + + def bzero : Checker<"bzero">, + HelpText<"Warn on uses of the 'bzero' function">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; + + def gets : Checker<"gets">, + HelpText<"Warn on uses of the 'gets' function">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; + + def getpw : Checker<"getpw">, + HelpText<"Warn on uses of the 'getpw' function">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; + + def mktemp : Checker<"mktemp">, + HelpText<"Warn on uses of the 'mktemp' function">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; + + def mkstemp + : Checker<"mkstemp">, + HelpText<"Warn when 'mkstemp' is passed fewer than 6 X's in the format " + "string">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; -def rand : Checker<"rand">, - HelpText<"Warn on uses of the 'rand', 'random', and related functions">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; + def rand + : Checker<"rand">, + HelpText<"Warn on uses of the 'rand', 'random', and related functions">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; -def strcpy : Checker<"strcpy">, - HelpText<"Warn on uses of the 'strcpy' and 'strcat' functions">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; + def strcpy : Checker<"strcpy">, + HelpText<"Warn on uses of the 'strcpy' and 'strcat' functions">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; -def vfork : Checker<"vfork">, - HelpText<"Warn on uses of the 'vfork' function">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; + def vfork : Checker<"vfork">, + HelpText<"Warn on uses of the 'vfork' function">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; -def UncheckedReturn : Checker<"UncheckedReturn">, - HelpText<"Warn on uses of functions whose return values must be always " - "checked">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; + def UncheckedReturn + : Checker<"UncheckedReturn">, + HelpText<"Warn on uses of functions whose return values must be always " + "checked">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; -def DeprecatedOrUnsafeBufferHandling : - Checker<"DeprecatedOrUnsafeBufferHandling">, - HelpText<"Warn on uses of unsecure or deprecated buffer manipulating " - "functions">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; + def DeprecatedOrUnsafeBufferHandling + : Checker<"DeprecatedOrUnsafeBufferHandling">, + HelpText<"Warn on uses of unsecure or deprecated buffer manipulating " + "functions">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; -def decodeValueOfObjCType : Checker<"decodeValueOfObjCType">, - HelpText<"Warn on uses of the '-decodeValueOfObjCType:at:' method">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; + def decodeValueOfObjCType + : Checker<"decodeValueOfObjCType">, + HelpText<"Warn on uses of the '-decodeValueOfObjCType:at:' method">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; } // end "security.insecureAPI" diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 5e75c1c4a3abd..5415e19431ca5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -74,13 +74,14 @@ class WalkAST : public StmtVisitor { 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 +102,7 @@ class WalkAST : public StmtVisitor { 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 +177,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 +542,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_getpw, + "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 +1098,30 @@ namespace { class SecuritySyntaxChecker : public Checker { 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 *checker = mgr.registerChecker(); + StringRef WarnOption = + mgr.getAnalyzerOptions().getCheckerStringOption(checker, "Warn"); + checker->setWarnFunctions(WarnOption); } bool ento::shouldRegisterSecuritySyntaxChecker(const CheckerManager &mgr) { diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 7936273415ad4..88b0ee427404a 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/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