diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 429c334a0b24b..1cee216ef97f9 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1056,18 +1056,6 @@ def MallocOverflowSecurityChecker : Checker<"MallocOverflow">, def MmapWriteExecChecker : Checker<"MmapWriteExec">, HelpText<"Warn on mmap() calls that are both writable and executable">, - CheckerOptions<[ - CmdLineOption, - CmdLineOption - ]>, Documentation; def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">, diff --git a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp index cd1dd1b2fc511..4b8e5216550d9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp @@ -21,49 +21,56 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" using namespace clang; using namespace ento; namespace { -class MmapWriteExecChecker : public Checker { +class MmapWriteExecChecker + : public Checker, check::PreCall> { CallDescription MmapFn{CDM::CLibrary, {"mmap"}, 6}; CallDescription MprotectFn{CDM::CLibrary, {"mprotect"}, 3}; - static int ProtWrite; - static int ProtExec; - static int ProtRead; const BugType BT{this, "W^X check fails, Write Exec prot flags set", "Security"}; + // Default values are used if definition of the flags is not found. + mutable int ProtRead = 0x01; + mutable int ProtWrite = 0x02; + mutable int ProtExec = 0x04; + public: + void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr, + BugReporter &BR) const; void checkPreCall(const CallEvent &Call, CheckerContext &C) const; - int ProtExecOv; - int ProtReadOv; }; } -int MmapWriteExecChecker::ProtWrite = 0x02; -int MmapWriteExecChecker::ProtExec = 0x04; -int MmapWriteExecChecker::ProtRead = 0x01; +void MmapWriteExecChecker::checkASTDecl(const TranslationUnitDecl *TU, + AnalysisManager &Mgr, + BugReporter &BR) const { + Preprocessor &PP = Mgr.getPreprocessor(); + const std::optional FoundProtRead = tryExpandAsInteger("PROT_READ", PP); + const std::optional FoundProtWrite = + tryExpandAsInteger("PROT_WRITE", PP); + const std::optional FoundProtExec = tryExpandAsInteger("PROT_EXEC", PP); + if (FoundProtRead && FoundProtWrite && FoundProtExec) { + ProtRead = *FoundProtRead; + ProtWrite = *FoundProtWrite; + ProtExec = *FoundProtExec; + } +} void MmapWriteExecChecker::checkPreCall(const CallEvent &Call, - CheckerContext &C) const { + CheckerContext &C) const { if (matchesAny(Call, MmapFn, MprotectFn)) { SVal ProtVal = Call.getArgSVal(2); auto ProtLoc = ProtVal.getAs(); if (!ProtLoc) return; int64_t Prot = ProtLoc->getValue().getSExtValue(); - if (ProtExecOv != ProtExec) - ProtExec = ProtExecOv; - if (ProtReadOv != ProtRead) - ProtRead = ProtReadOv; - - // Wrong settings - if (ProtRead == ProtExec) - return; - if ((Prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) { + if ((Prot & ProtWrite) && (Prot & ProtExec)) { ExplodedNode *N = C.generateNonFatalErrorNode(); if (!N) return; @@ -80,17 +87,10 @@ void MmapWriteExecChecker::checkPreCall(const CallEvent &Call, } } -void ento::registerMmapWriteExecChecker(CheckerManager &mgr) { - MmapWriteExecChecker *Mwec = - mgr.registerChecker(); - Mwec->ProtExecOv = - mgr.getAnalyzerOptions() - .getCheckerIntegerOption(Mwec, "MmapProtExec"); - Mwec->ProtReadOv = - mgr.getAnalyzerOptions() - .getCheckerIntegerOption(Mwec, "MmapProtRead"); +void ento::registerMmapWriteExecChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); } -bool ento::shouldRegisterMmapWriteExecChecker(const CheckerManager &mgr) { +bool ento::shouldRegisterMmapWriteExecChecker(const CheckerManager &) { return true; } diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index fda920fa3951e..7d29270222517 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -9,8 +9,6 @@ // CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true // CHECK-NEXT: alpha.cplusplus.STLAlgorithmModeling:AggressiveStdFindModeling = false // CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false -// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04 -// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01 // CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = "" // CHECK-NEXT: apply-fixits = false // CHECK-NEXT: assume-controlled-environment = false diff --git a/clang/test/Analysis/mmap-writeexec.c b/clang/test/Analysis/mmap-writeexec.c index 8fd86ceb9d2a2..579cc75069eec 100644 --- a/clang/test/Analysis/mmap-writeexec.c +++ b/clang/test/Analysis/mmap-writeexec.c @@ -1,13 +1,14 @@ -// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=alpha.security.MmapWriteExec -analyzer-config alpha.security.MmapWriteExec:MmapProtExec=1 -analyzer-config alpha.security.MmapWriteExec:MmapProtRead=4 -DUSE_ALTERNATIVE_PROT_EXEC_DEFINITION -verify %s +// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=alpha.security.MmapWriteExec -DUSE_ALTERNATIVE_PROT_EXEC_DEFINITION -verify %s // RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=alpha.security.MmapWriteExec -verify %s -#define PROT_WRITE 0x02 #ifndef USE_ALTERNATIVE_PROT_EXEC_DEFINITION -#define PROT_EXEC 0x04 -#define PROT_READ 0x01 -#else #define PROT_EXEC 0x01 +#define PROT_WRITE 0x02 #define PROT_READ 0x04 +#else +#define PROT_EXEC 0x08 +#define PROT_WRITE 0x04 +#define PROT_READ 0x02 #endif #define MAP_PRIVATE 0x0002 #define MAP_ANON 0x1000