From: Ted Kremenek Date: Fri, 20 Jan 2012 01:44:29 +0000 (+0000) Subject: Turn 'SecuritySyntaxChecker' into a "meta" security checker for insecure APIs. Now X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=76a54246dbbe6cc3c74186e64f8ea0deb4a64ea2;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@148525 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 06899fc578..6f55d164a7 100644 --- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/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 { BugReporter &BR; AnalysisDeclContext* AC; @@ -41,11 +58,14 @@ class WalkAST : public StmtVisitor { 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(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(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(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 { 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(); +#define REGISTER_CHECKER(name) \ +void ento::register##name(CheckerManager &mgr) {\ + mgr.registerChecker()->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) + diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index 5dc4d30ec8..fe22e8487c 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -27,6 +27,7 @@ def DeadCode : Package<"deadcode">; def DeadCodeExperimental : Package<"deadcode">, InPackage, Hidden; def Security : Package <"security">; +def InsecureAPI : Package<"insecureAPI">, InPackage; def SecurityExperimental : Package<"security">, InPackage, Hidden; def Taint : Package<"taint">, InPackage, Hidden; @@ -198,11 +199,33 @@ def UnreachableCodeChecker : Checker<"UnreachableCode">, // Security checkers. //===----------------------------------------------------------------------===// -let ParentPackage = SecurityExperimental in { +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">; +} -def SecuritySyntaxChecker : Checker<"SecuritySyntactic">, - HelpText<"Perform quick security API checks that require no data flow">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; +let ParentPackage = SecurityExperimental in { def ArrayBoundChecker : Checker<"ArrayBound">, HelpText<"Warn about buffer overflows (older checker)">, diff --git a/test/Analysis/security-syntax-checks-no-emit.c b/test/Analysis/security-syntax-checks-no-emit.c index cbd432a19b..c2869cabae 100644 --- a/test/Analysis/security-syntax-checks-no-emit.c +++ b/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. diff --git a/test/Analysis/security-syntax-checks.m b/test/Analysis/security-syntax-checks.m index a04401b531..7c0cda3c79 100644 --- a/test/Analysis/security-syntax-checks.m +++ b/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