[clang] [clang][analyzer] MmapWriteExecChecker improvements (PR #97078)

[Balázs Kéri via cfe-commits]

https://github.com/balazske created https://github.com/llvm/llvm-project/pull/97078

Read the 'mmap' flags from macro values and use a better test for the error situation.

>From 1f04ce794a3aefc0f5622a9dea0a92a1e2b50be9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Tue, 25 Jun 2024 16:27:00 +0200
Subject: [PATCH] [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