[analyzer] Add a checker for mmap()s which are both writable and executable.

This is a security check which is disabled by default but will be enabled
whenever the user consciously enables the security package. If mmap()ed memory
is both writable and executable, it makes it easier for the attacker to execute
arbitrary code when contents of this memory are compromised. Some applications
require such mmap()s though, such as different sorts of JIT.

Patch by David Carlier!

Differential Revision: https://reviews.llvm.org/D42645

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@324166 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Checkers/Checkers.td b/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 77820aeea30c99a72e9e33aad51d61530af4796a..e04649cc8cb7628b668f0ce025c0df51115b195a 100644 (file)
--- a/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -394,6 +394,10 @@ 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 MmapWriteExecChecker : Checker<"MmapWriteExec">,
+    HelpText<"Check if mmap() call is not both writable and executable">,
+    DescFile<"MmapWriteExecChecker.cpp">;
 }
 
 let ParentPackage = SecurityAlpha in {

diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 7ab9c6114eae3bfca4549975e0ade7519d40eb9b..1137032aacc1b6814d7163d473c292664b02fb45 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -50,6 +50,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   MallocOverflowSecurityChecker.cpp
   MallocSizeofChecker.cpp
   MisusedMovedObjectChecker.cpp
+  MmapWriteExecChecker.cpp
   MPI-Checker/MPIBugReporter.cpp
   MPI-Checker/MPIChecker.cpp
   MPI-Checker/MPIFunctionClassifier.cpp

diff --git a/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
new file mode 100644 (file)
index 0000000..5dcac20
--- /dev/null
+++ b/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,75 @@
+// MmapWriteExecChecker.cpp - Check for the prot argument -----------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This checker tests the 3rd argument of mmap's calls to check if
+// it is writable and executable in the same time. It's somehow
+// an optional checker since for example in JIT libraries it is pretty common.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+using llvm::APSInt;
+
+namespace {
+class MmapWriteExecChecker : public Checker<check::PreCall> {
+  CallDescription MmapFn;
+  static int ProtWrite;
+  static int ProtExec;
+  mutable std::unique_ptr<BugType> BT;
+public:
+  MmapWriteExecChecker() : MmapFn("mmap", 6) {}
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+};
+}
+
+int MmapWriteExecChecker::ProtWrite = 0x02;
+int MmapWriteExecChecker::ProtExec  = 0x04;
+
+void MmapWriteExecChecker::checkPreCall(const CallEvent &Call,
+                                         CheckerContext &C) const {
+  if (Call.isCalled(MmapFn)) {
+    llvm::Triple Triple = C.getASTContext().getTargetInfo().getTriple();
+
+    if (Triple.isOSGlibc())
+      ProtExec = 0x01;
+
+    SVal ProtVal = Call.getArgSVal(2); 
+    Optional<nonloc::ConcreteInt> ProtLoc = ProtVal.getAs<nonloc::ConcreteInt>();
+    int64_t Prot = ProtLoc->getValue().getSExtValue();
+
+    if ((Prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
+      if (!BT)
+        BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security"));
+
+      ExplodedNode *N = C.generateNonFatalErrorNode();
+      if (!N)
+        return;
+
+      auto Report = llvm::make_unique<BugReport>(
+          *BT, "Both PROT_WRITE and PROT_EXEC flags had been set. It can "
+               "lead to exploitable memory regions, overwritten with malicious code"
+         , N);
+      Report->addRange(Call.getArgSourceRange(2));
+      C.emitReport(std::move(Report));
+    }
+  }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+  mgr.registerChecker<MmapWriteExecChecker>();
+}

diff --git a/test/Analysis/mmap-writeexec.c b/test/Analysis/mmap-writeexec.c
new file mode 100644 (file)
index 0000000..071eb2a
--- /dev/null
+++ b/test/Analysis/mmap-writeexec.c
@@ -0,0 +1,27 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_READ   0x01
+#define PROT_WRITE  0x02
+#define PROT_EXEC   0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON    0x1000
+#define MAP_FIXED   0x0010
+#define NULL        ((void *)0)
+
+typedef __typeof(sizeof(int)) size_t;
+void *mmap(void *, size_t, int, int, int, long);
+
+void f1()
+{
+  void *a = mmap(NULL, 16, PROT_READ | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); // no-warning
+  void *b = mmap(a, 16, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FIXED | MAP_ANON, -1, 0); // no-warning
+  void *c = mmap(NULL, 32, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); // expected-warning{{Both PROT_WRITE and PROT_EXEC flags had been set. It can lead to exploitable memory regions, overwritten with malicious code}}
+}
+
+void f2()
+{
+  void *(*callm)(void *, size_t, int, int, int, long);
+  callm = mmap;
+  int prot = PROT_WRITE | PROT_EXEC;
+  (void)callm(NULL, 1024, prot, MAP_PRIVATE | MAP_ANON, -1, 0); // expected-warning{{Both PROT_WRITE and PROT_EXEC flags had been set. It can lead to exploitable memory regions, overwritten with malicious code}}
+}