https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/97078
From 1f04ce794a3aefc0f5622a9dea0a92a1e2b50be9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Tue, 25 Jun 2024 16:27:00 +0200 Subject: [PATCH 1/2] [clang][analyzer] MmapWriteExecChecker improvements Read the 'mmap' flags from macro values and use a better test for the error situation . --- .../Checkers/MmapWriteExecChecker.cpp | 49 +++++++++++++------ clang/test/Analysis/mmap-writeexec.c | 9 ++-- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp index cd1dd1b2fc511..a5a2bd62b47d6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp @@ -21,30 +21,55 @@ #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<check::PreCall> { +class MmapWriteExecChecker + : public Checker<check::BeginFunction, 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"}; + mutable bool FlagsInitialized = false; + mutable int ProtRead = 0x01; + mutable int ProtWrite = 0x02; + mutable int ProtExec = 0x04; + public: + void checkBeginFunction(CheckerContext &C) 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::checkBeginFunction(CheckerContext &C) const { + if (FlagsInitialized) + return; + + FlagsInitialized = true; + + const std::optional<int> FoundProtRead = + tryExpandAsInteger("PROT_READ", C.getPreprocessor()); + const std::optional<int> FoundProtWrite = + tryExpandAsInteger("PROT_WRITE", C.getPreprocessor()); + const std::optional<int> FoundProtExec = + tryExpandAsInteger("PROT_EXEC", C.getPreprocessor()); + if (FoundProtRead && FoundProtWrite && FoundProtExec) { + ProtRead = *FoundProtRead; + ProtWrite = *FoundProtWrite; + ProtExec = *FoundProtExec; + } else { + // FIXME: Are these useful? + ProtRead = ProtReadOv; + ProtExec = ProtExecOv; + } +} void MmapWriteExecChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { @@ -54,16 +79,8 @@ void MmapWriteExecChecker::checkPreCall(const CallEvent &Call, 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; diff --git a/clang/test/Analysis/mmap-writeexec.c b/clang/test/Analysis/mmap-writeexec.c index 8fd86ceb9d2a2..88fcc7788e683 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 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 From dbdfe92c4b89e4af1bd549fe07a4743c05685286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Tue, 23 Jul 2024 17:49:54 +0200 Subject: [PATCH 2/2] using checkASTDecl, added PP variable, removed options --- .../clang/StaticAnalyzer/Checkers/Checkers.td | 12 ----- .../Checkers/MmapWriteExecChecker.cpp | 47 ++++++------------- 2 files changed, 15 insertions(+), 44 deletions(-) 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<Integer, - "MmapProtExec", - "Specifies the value of PROT_EXEC", - "0x04", - Released>, - CmdLineOption<Integer, - "MmapProtRead", - "Specifies the value of PROT_READ", - "0x01", - Released> - ]>, Documentation<HasDocumentation>; def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">, diff --git a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp index a5a2bd62b47d6..4b8e5216550d9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp @@ -28,51 +28,41 @@ using namespace ento; namespace { class MmapWriteExecChecker - : public Checker<check::BeginFunction, check::PreCall> { + : public Checker<check::ASTDecl<TranslationUnitDecl>, check::PreCall> { CallDescription MmapFn{CDM::CLibrary, {"mmap"}, 6}; CallDescription MprotectFn{CDM::CLibrary, {"mprotect"}, 3}; const BugType BT{this, "W^X check fails, Write Exec prot flags set", "Security"}; - mutable bool FlagsInitialized = false; + // 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 checkBeginFunction(CheckerContext &C) const; + void checkASTDecl(const TranslationUnitDecl *TU, AnalysisManager &Mgr, + BugReporter &BR) const; void checkPreCall(const CallEvent &Call, CheckerContext &C) const; - - int ProtExecOv; - int ProtReadOv; }; } -void MmapWriteExecChecker::checkBeginFunction(CheckerContext &C) const { - if (FlagsInitialized) - return; - - FlagsInitialized = true; - - const std::optional<int> FoundProtRead = - tryExpandAsInteger("PROT_READ", C.getPreprocessor()); +void MmapWriteExecChecker::checkASTDecl(const TranslationUnitDecl *TU, + AnalysisManager &Mgr, + BugReporter &BR) const { + Preprocessor &PP = Mgr.getPreprocessor(); + const std::optional<int> FoundProtRead = tryExpandAsInteger("PROT_READ", PP); const std::optional<int> FoundProtWrite = - tryExpandAsInteger("PROT_WRITE", C.getPreprocessor()); - const std::optional<int> FoundProtExec = - tryExpandAsInteger("PROT_EXEC", C.getPreprocessor()); + tryExpandAsInteger("PROT_WRITE", PP); + const std::optional<int> FoundProtExec = tryExpandAsInteger("PROT_EXEC", PP); if (FoundProtRead && FoundProtWrite && FoundProtExec) { ProtRead = *FoundProtRead; ProtWrite = *FoundProtWrite; ProtExec = *FoundProtExec; - } else { - // FIXME: Are these useful? - ProtRead = ProtReadOv; - ProtExec = ProtExecOv; } } 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<nonloc::ConcreteInt>(); @@ -97,17 +87,10 @@ void MmapWriteExecChecker::checkPreCall(const CallEvent &Call, } } -void ento::registerMmapWriteExecChecker(CheckerManager &mgr) { - MmapWriteExecChecker *Mwec = - mgr.registerChecker<MmapWriteExecChecker>(); - Mwec->ProtExecOv = - mgr.getAnalyzerOptions() - .getCheckerIntegerOption(Mwec, "MmapProtExec"); - Mwec->ProtReadOv = - mgr.getAnalyzerOptions() - .getCheckerIntegerOption(Mwec, "MmapProtRead"); +void ento::registerMmapWriteExecChecker(CheckerManager &Mgr) { + Mgr.registerChecker<MmapWriteExecChecker>(); } -bool ento::shouldRegisterMmapWriteExecChecker(const CheckerManager &mgr) { +bool ento::shouldRegisterMmapWriteExecChecker(const CheckerManager &) { return true; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits