From: Artem Dergachev Date: Sat, 3 Feb 2018 02:33:42 +0000 (+0000) Subject: [analyzer] Add a checker for mmap()s which are both writable and executable. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=726a6da4d194338af2b8455cb6fbd27ede7d4b9c;p=clang [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 77820aeea3..e04649cc8c 100644 --- 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 7ab9c6114e..1137032aac 100644 --- 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 index 0000000000..5dcac20180 --- /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 { + CallDescription MmapFn; + static int ProtWrite; + static int ProtExec; + mutable std::unique_ptr 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 ProtLoc = ProtVal.getAs(); + 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( + *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(); +} diff --git a/test/Analysis/mmap-writeexec.c b/test/Analysis/mmap-writeexec.c new file mode 100644 index 0000000000..071eb2a397 --- /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}} +}