[PATCH] D42645: New simple Checker for mmap calls
alexfh added inline comments. Herald added a subscriber: jdoerfert. Herald added a project: LLVM. Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:31-33 + static int ProtWrite; + static int ProtExec; + static int ProtRead; Can these be non-static members instead? These are one of just a few uses of static fields in Clang. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42645/new/ https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
NoQ added a comment. Sorry, busy days>< Done. Repository: rL LLVM https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
This revision was automatically updated to reflect the committed changes.
Closed by commit rL326405: [analyzer] Add a checker for mmap()s which are both
writable and executable. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D42645?vs=135258&id=136449#toc
Repository:
rL LLVM
https://reviews.llvm.org/D42645
Files:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
cfe/trunk/test/Analysis/mmap-writeexec.c
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
MisusedMovedObjectChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,87 @@
+// 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;
+ static int ProtRead;
+ mutable std::unique_ptr BT;
+public:
+ MmapWriteExecChecker() : MmapFn("mmap", 6) {}
+ 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::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (Call.isCalled(MmapFn)) {
+SVal ProtVal = Call.getArgSVal(2);
+Optional ProtLoc = ProtVal.getAs();
+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 (!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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten "
+ "with malicious code", N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ MmapWriteExecChecker *Mwec =
+ mgr.registerChecker();
+ Mwec->ProtExecOv =
+mgr.getAnalyzerOptions().getOptionAsInteger("MmapProtExec", 0x04, Mwec);
+ Mwec->ProtReadOv =
+mgr.getAnalyzerOptions().getOptionAsInteger("MmapProtRead", 0x01, Mwec);
+}
Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -414,6 +414,13 @@
HelpText<"Check for overflows in the arguments to malloc()">,
DescFile<"MallocOverflowSecurityChecker.cpp">;
+// Operating systems specific PROT_READ/PROT_WRITE values is not implemented,
+// the defaults are correct for several common operating systems though,
+// but may need to be overridden via the
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. ping :) https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thank you! I'll try to commit again. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 135258.
devnexen added a comment.
Rephrasing Checkers.td comment
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,36 @@
+// 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_READ 0x04
+#endif
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,87 @@
+// 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;
+ static int ProtRead;
+ mutable std::unique_ptr BT;
+public:
+ MmapWriteExecChecker() : MmapFn("mmap", 6) {}
+ 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::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (Call.isCalled(MmapFn)) {
+SVal ProtVal = Call.getArgSVal(2);
+Optional ProtLoc = ProtVal.getAs();
+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 (!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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten "
+ "
[PATCH] D42645: New simple Checker for mmap calls
devnexen added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419 +// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, +// thus ought to be overriden with the proper analyser-config variables +// remain in alpha until the state changes emaste wrote: > devnexen wrote: > > emaste wrote: > > > I'm a bit confused by this comment; this checker works as-is for most > > > common operating system cases, correct? > > Most of them yes, at least Muslc linux most of glibc I tested too. Not to > > mention *BSD ... But might be safer to put it as alpha for a start. > OK - to me it implies that the checker only works (anywhere) if the user > provides the flag values. Maybe something like "the defaults are correct for > several common operating systems, but may need to be overridden " Fair point, I ll rephrase a bit. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
emaste added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419 +// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, +// thus ought to be overriden with the proper analyser-config variables +// remain in alpha until the state changes devnexen wrote: > emaste wrote: > > I'm a bit confused by this comment; this checker works as-is for most > > common operating system cases, correct? > Most of them yes, at least Muslc linux most of glibc I tested too. Not to > mention *BSD ... But might be safer to put it as alpha for a start. OK - to me it implies that the checker only works (anywhere) if the user provides the flag values. Maybe something like "the defaults are correct for several common operating systems, but may need to be overridden " Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:64 + if (!BT) +BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security")); + devnexen wrote: > emaste wrote: > > `Write & Exec` (or `Write and Exec`) perhaps (assuming it doesn't become > > over-long then)? > I kept short intentionally indeed we can always change but the user in order > to use it needs to enable it willingly so I assumed the user might know > enough about the topic in question. Understood. To me it just read as "Write Exec" as one entity. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419 +// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, +// thus ought to be overriden with the proper analyser-config variables +// remain in alpha until the state changes emaste wrote: > I'm a bit confused by this comment; this checker works as-is for most common > operating system cases, correct? Most of them yes, at least Muslc linux most of glibc I tested too. Not to mention *BSD ... But might be safer to put it as alpha for a start. Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:64 + if (!BT) +BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security")); + emaste wrote: > `Write & Exec` (or `Write and Exec`) perhaps (assuming it doesn't become > over-long then)? I kept short intentionally indeed we can always change but the user in order to use it needs to enable it willingly so I assumed the user might know enough about the topic in question. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
emaste added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:417-419 +// Operating systems specific PROT_READ/PROT_WRITE values is not implemented, +// thus ought to be overriden with the proper analyser-config variables +// remain in alpha until the state changes I'm a bit confused by this comment; this checker works as-is for most common operating system cases, correct? Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:64 + if (!BT) +BT.reset(new BugType(this, "W^X check fails, Write Exec prot flags set", "Security")); + `Write & Exec` (or `Write and Exec`) perhaps (assuming it doesn't become over-long then)? https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
a.sidorin accepted this revision. a.sidorin added a comment. Looks good to me as an 'alpha' checker. Thank you David! I'd prefer this patch to be accepted by somebody else as well before committing it. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 135067.
devnexen added a comment.
Updating tests accordingly
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,36 @@
+// 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_READ 0x04
+#endif
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -414,6 +414,13 @@
HelpText<"Check for overflows in the arguments to malloc()">,
DescFile<"MallocOverflowSecurityChecker.cpp">;
+// Operating systems specific PROT_READ/PROT_WRITE values is not implemented,
+// thus ought to be overriden with the proper analyser-config variables
+// remain in alpha until the state changes
+def MmapWriteExecChecker : Checker<"MmapWriteExec">,
+ HelpText<"Warn on mmap() calls that are both writable and executable">,
+ DescFile<"MmapWriteExecChecker.cpp">;
+
} // end "alpha.security"
//===--===//
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,87 @@
+// 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;
+ static int ProtRead;
+ mutable std::unique_ptr BT;
+public:
+ MmapWriteExecChecker() : MmapFn("mmap", 6) {}
+ 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::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (Ca
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 135027.
devnexen added a comment.
Moving back the checker to alpha.security level.
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=security.MmapWriteExec -analyzer-config security.MmapWriteExec:MmapProtExec=1 -analyzer-config security.MmapWriteExec:MmapProtRead=4 -DUSE_ALTERNATIVE_PROT_EXEC_DEFINITION -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=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_READ 0x04
+#endif
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,87 @@
+// 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;
+ static int ProtRead;
+ mutable std::unique_ptr BT;
+public:
+ MmapWriteExecChecker() : MmapFn("mmap", 6) {}
+ 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::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (Call.isCalled(MmapFn)) {
+SVal ProtVal = Call.getArgSVal(2);
+Optional ProtLoc = ProtVal.getAs();
+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 (!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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten "
+ "with m
[PATCH] D42645: New simple Checker for mmap calls
a.sidorin added a comment. Hello David, There are still some items for improvement. However, if we move this checker into 'alpha' category, as Artem pointed, I think it can be accepted for merge in its current state and improved later. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
NoQ added a comment. I believe we should relocate this checker into `alpha.security` in order to indicate that this is still in development, so that you (or anyone else) could provide auto-detection for the macro values later as an incremental improvement, and then it will be back in `security`. A comment should be added to explain that autodetection is not currently implemented, and that the default values are not correct for all platforms, and that this is the only reason why this is considered to be alpha. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. ping Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 133564.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=security.MmapWriteExec -analyzer-config security.MmapWriteExec:MmapProtExec=1 -analyzer-config security.MmapWriteExec:MmapProtRead=4 -DUSE_ALTERNATIVE_PROT_EXEC_DEFINITION -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=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_READ 0x04
+#endif
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,87 @@
+// 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;
+ static int ProtRead;
+ mutable std::unique_ptr BT;
+public:
+ MmapWriteExecChecker() : MmapFn("mmap", 6) {}
+ 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::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (Call.isCalled(MmapFn)) {
+SVal ProtVal = Call.getArgSVal(2);
+Optional ProtLoc = ProtVal.getAs();
+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 (!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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten "
+ "with malicious code", N);
+ Report->addRange(Call.getA
[PATCH] D42645: New simple Checker for mmap calls
NoQ added a comment.
Hmm, maybe it'd also be a good idea to disable the check completely when a
likely-correct value for the macro cannot be determined.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:74-75
+ mgr.registerChecker();
+ Mwec->ProtExecOv =
+mgr.getAnalyzerOptions().getOptionAsInteger("MmapProtExec", 0x04, Mwec);
+}
You'd probably want to be able to set both `PROT_READ` and `PROT_EXEC`, to be
fully flexible.
Comment at: test/Analysis/mmap-writeexec.c:1
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux
-analyzer-checker=security.MmapWriteExec -analyzer-config
security.MmapWriteExec:MmapProtExec=1 -DPROT_EXEC=1 -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10
-analyzer-checker=security.MmapWriteExec -verify %s
I think you should use an unrelated macro, eg. `-analyzer-config
security.MmapWriteExec:MmapProtExec=1 -DUSE_ALTERNATIVE_PROT_EXEC_DEFINITION`,
to trigger different definitions in the tests, i.e.
```
#ifndef USE_ALTERNATIVE_PROT_EXEC_DEFINITION
#define PROT_READ 0x01
#define PROT_EXEC 0x04
#else
#define PROT_READ 0x04
#define PROT_EXEC 0x01
#endif
```
Repository:
rC Clang
https://reviews.llvm.org/D42645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. None of the possible solutions are ideal, but I think I chose the least complex (e.g. via analyzer-config), less edgy one, and 4 is the most common value I ve found so far for PROT_EXEC. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 133164.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=security.MmapWriteExec -analyzer-config security.MmapWriteExec:MmapProtExec=1 -DPROT_EXEC=1 -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_WRITE 0x02
+#ifndef PROT_EXEC
+#define PROT_EXEC 0x04
+#define PROT_READ 0x01
+#else
+#define PROT_READ 0x04
+#endif
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,76 @@
+// 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 ProtExecOv;
+};
+}
+
+int MmapWriteExecChecker::ProtWrite = 0x02;
+int MmapWriteExecChecker::ProtExec = 0x04;
+
+void MmapWriteExecChecker::checkPreCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ if (Call.isCalled(MmapFn)) {
+SVal ProtVal = Call.getArgSVal(2);
+Optional ProtLoc = ProtVal.getAs();
+int64_t Prot = ProtLoc->getValue().getSExtValue();
+if (ProtExecOv != ProtExec)
+ ProtExec = ProtExecOv;
+
+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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten "
+ "with malicious code", N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ MmapWriteExecChecker *Mwec =
+ mgr.registerChecker();
+ Mwec->ProtExecOv =
+mgr.getAnalyzerOptions().getOptionAsInteger("MmapProtExec", 0x04, Mwec);
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
[PATCH] D42645: New simple Checker for mmap calls
NoQ added a comment. > What I propose is "two" separated checkers one using same but the value of > PROT_EXEC will depend on the checker name e.g. security.MmapWriteExecGen vs > security.MmapWriteExecGlibc ... or by default we keep PROT_EXEC as is and we > would allow to override the value via analyser options. I would go with the > second option. This doesn't sound great; most of the users wouldn't ever go that far or read that much documentation to work correctly set these values. Analyzer config, like alpha checkers, is mostly for disabling stuff that is still in development, or for playing with values during development and debugging. I'd rather try to either cover all cases, or look for the actual macro value. Like, if you have your `PROT_WRITE | PROT_EXEC` expression, it would look like `0x02 | 0x04` in the AST, but source locations of `0x02` and `0x04` would know that they came from macros with a certain name, which you could match. That, however, would defeat the purpose of path-sensitive analysis (flags would no longer be able to be located anywhere in the AST, i.e. passed through variables; tracking the value through assignments is possible but feels horribly wrong). I keep saying that in fact not using the path-sensitive engine is not bad and might be even good for this checker because these flags are hopefully rarely passed through variables, so you'd lose less positives due to not supporting pass through variables than you'd gain by not relying on the path sensitive engine's imperfect code coverage (large chunks of the code are not analyzed path-sensitively at all because the code before it was too complex to make any sense out of it). It should also be possible (but also unpleasant) to scan the whole AST to find `PROT_WRITE`-wrapped or `PROT_EXEC`-wrapped integer literals and see their values. Or you could probably pass some preprocessor info to the analyzer and see if you could find your answers there. So this is a long-standing problem without good solutions, unfortunately. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 133010.
devnexen added a comment.
Will work on most modern Linux/Glibc versions, BSD variants and Illumos.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/Driver/ToolChains/FreeBSD.cpp
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_WRITE 0x02
+#define PROT_EXEC 0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); // no-warning
+ void *b = mmap(a, 16, PROT_WRITE, MAP_PRIVATE | MAP_FIXED | MAP_ANON, -1, 0); // no-warning
+ void *c = mmap(NULL, 32, PROT_WRITE | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); // expected-warning{{Both PROT_WRITE and PROT_EXEC flags are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,70 @@
+// 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)) {
+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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten"
+ " with malicious code", N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ Mmap
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 133008.
devnexen added a comment.
Both Linux/Darwin unit tests passed.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/Driver/ToolChains/FreeBSD.cpp
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_WRITE 0x02
+#define PROT_EXEC 0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); // no-warning
+ void *b = mmap(a, 16, PROT_WRITE, MAP_PRIVATE | MAP_FIXED | MAP_ANON, -1, 0); // no-warning
+ void *c = mmap(NULL, 32, PROT_WRITE | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); // expected-warning{{Both PROT_WRITE and PROT_EXEC flags are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,72 @@
+// 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();
+
+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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten"
+ " with malicious code", N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecke
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132969.
Herald added a subscriber: emaste.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/Driver/ToolChains/FreeBSD.cpp
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_WRITE 0x02
+#ifdef __GLIBC__
+#define PROT_READ 0x04
+#define PROT_EXEC 0x01
+#else
+#define PROT_READ 0x01
+#define PROT_EXEC 0x04
+#endif
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten"
+ " with malicious code", N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. In https://reviews.llvm.org/D42645#998732, @a.sidorin wrote: > Hello David, > > I have looked into mmap constant definitions in different implementations and > found them pretty inconsistent. For example, MMAP_EXEC can be 0x01, 0x04 and > I even found 0x00 in some file > (https://www.cs.cmu.edu/~dga/crypto/priveth/libethash/mmap.h). Therefore, we > should clearly state how do we predict these values. Are you sure that > checking `isOSGlibc()` is enough? > > Also, could you please explain me how the test works? If I understand > correctly, for all platforms we manually define the constants in the test. > Then, we check if `PROT_WRITE | PROT_EXEC` is set. For OSGlibc, PROT_EXEC > is defined as 0x01 in the checker. This means that if isOSGlibc branch is > covered, we should not get any warnings for one of test launches because > `PROT_WRITE | PROT_EXEC` is 0x03 in the checker and is 0x06 in the test file. Yes maybe in the test glibc constants should be defined as well (I develop mainly on *BSD variants I missed that for the test case you re right). Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
a.sidorin added a comment. Hello David, I have looked into mmap constant definitions in different implementations and found them pretty inconsistent. For example, MMAP_EXEC can be 0x01, 0x04 and I even found 0x00 in some file (https://www.cs.cmu.edu/~dga/crypto/priveth/libethash/mmap.h). Therefore, we should clearly state how do we predict these values. Are you sure that checking `isOSGlibc()` is enough? Also, could you please explain me how the test works? If I understand correctly, for all platforms we manually define the constants in the test. Then, we check if `PROT_WRITE | PROT_EXEC` is set. For OSGlibc, PROT_EXEC is defined as 0x01 in the checker. This means that if isOSGlibc branch is covered, we should not get any warnings for one of test launches because `PROT_WRITE | PROT_EXEC` is 0x03 in the checker and is 0x06 in the test file. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132873.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_READ 0x01
+#define PROT_WRITE 0x02
+#define PROT_EXEC 0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten "
+ "with malicious code", N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverfl
[PATCH] D42645: New simple Checker for mmap calls
a.sidorin added a comment. Hi David! The patch looks almost OK. Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:65 + *BT, "Both PROT_WRITE and PROT_EXEC flags are set. This can " + "lead to exploitable memory regions, which could be overwritten with malicious code" + , N); This line violates 80-char limit. Next line starts with comma which is not good. Comment at: test/Analysis/mmap-writeexec.c:1 +// RUN: %clang_analyze_cc1 -triple i686-unknown-freebsd -analyzer-checker=security.MmapWriteExec -verify %s +// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s Do you try to test `if (Triple.isOSGlibc())` branch here? If so, `i686-unknown-freebsd` doesn't look like an appropriate target (it is not kFreeBSD). You can use `-triple=x86_64-pc-kfreebsd-gnu` or just `i686-unknown-linux`. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. Correcting last typos in unit test. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132850.
devnexen edited the summary of this revision.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-freebsd -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_READ 0x01
+#define PROT_WRITE 0x02
+#define PROT_EXEC 0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+ (void)a;
+ (void)b;
+ (void)c;
+}
+
+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 are set. This can lead to exploitable memory regions, which could be overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten with malicious code"
+ , N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132829.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-freebsd -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_READ 0x01
+#define PROT_WRITE 0x02
+#define PROT_EXEC 0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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 are set. This can lead to exploitable memory regions, which could be 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 are set. This can lead to exploitable memory regions, overwritten with malicious code}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten with malicious code"
+ , N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ Mmap
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132826.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-freebsd -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_READ 0x01
+#define PROT_WRITE 0x02
+#define PROT_EXEC 0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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 are set. This can "
+ "lead to exploitable memory regions, which could be overwritten with malicious code"
+ , N);
+ Report->addRange(Call.getArgSourceRange(2));
+ C.emitReport(std::move(Report));
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecC
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. Your remarks make sense. Ok will update the general "tone" accordingly. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
jroelofs added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:399 + def MmapWriteExecChecker : Checker<"MmapWriteExec">, +HelpText<"Check if mmap() call is not both writable and executable">, +DescFile<"MmapWriteExecChecker.cpp">; I'd reword as: "Warn on mmap() calls that are both writeable and executable" Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:64 + 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" The general style of diagnostics is to write everything in the present tense. Talk about how the code is, as the compiler saw it, not how it was. I'd reword it as: "Both PROT_WRITE and PROT_EXEC are set. This can lead to exploitable memory regions, which could be overwritten with malicious code" Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. In https://reviews.llvm.org/D42645#997062, @NoQ wrote: > This failed on the buildbots (for example > http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/14910 - > committer gets notified of those), so i reverted it for now as r324167. > > I guess the reason is that buildbots have different triples, and even though > you check the target's triple, the test runs under the host's triple, which > would be different on different machines. You should add an explicit target > triple to the test run-line. It's also a good idea to have two run-lines with > different triples and make tests for both, if they'd cover different code > paths in your checker. Ah yes the cross-building aspect always gets me. Updated hope it will work this time. Apologies. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132728.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-freebsd -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_READ 0x01
+#define PROT_WRITE 0x02
+#define PROT_EXEC 0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132727.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ test/Analysis/mmap-writeexec.c
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -triple i686-unknown-freebsd -analyzer-checker=security.MmapWriteExec -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-apple-darwin10 -analyzer-checker=security.MmapWriteExec -verify %s
+
+#define PROT_READ 0x01
+#define PROT_WRITE 0x02
+#define PROT_EXEC 0x04
+#define MAP_PRIVATE 0x0002
+#define MAP_ANON0x1000
+#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}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
[PATCH] D42645: New simple Checker for mmap calls
NoQ added a comment. This failed on the buildbots (for example http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/14910 - committer gets notified of those), so i reverted it for now as r324167. I guess the reason is that buildbots have different triples, and even though you check the target's triple, the test runs under the host's triple, which would be different on different machines. You should add an explicit target triple to the test run-line. It's also a good idea to have two run-lines with different triples and make tests for both, if they'd cover different code paths in your checker. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
This revision was automatically updated to reflect the committed changes.
Closed by commit rC324166: [analyzer] Add a checker for mmap()s which are both
writable and executable. (authored by dergachev, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D42645?vs=132715&id=132719#toc
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ 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_ANON0x1000
+#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}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -50,6 +50,7 @@
Ma
[PATCH] D42645: New simple Checker for mmap calls
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Fantastic, thanks! I'll commit. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132715.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ 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_ANON0x1000
+#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}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
MisusedMovedObjectChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checke
[PATCH] D42645: New simple Checker for mmap calls
NoQ added a comment. Wonderful! Could you also add a `// no-warning` kind of test, i.e. when the respective flags are not set, no warning is emitted? Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. In https://reviews.llvm.org/D42645#996668, @NoQ wrote: > All right, so the code looks good now, but in order to commit this, we also > need tests. We've got those automatic tests of ours in `test/Analysis/` that > feed small code snippets into the analyzer and verify that it does (or > doesn't) emit warnings in certain cases, so you'd need to prove that your > checker works (as intended, ideally) by writing some of those > (http://clang-analyzer.llvm.org/checker_dev_manual.html#testing). Updated with a little new unit test. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132713.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
test/Analysis/mmap-writeexec.c
Index: test/Analysis/mmap-writeexec.c
===
--- test/Analysis/mmap-writeexec.c
+++ 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_ANON0x1000
+#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);
+ void *b = mmap(a, 16, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FIXED | MAP_ANON, -1, 0);
+ 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}}
+}
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
MisusedMovedObjectChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
==
[PATCH] D42645: New simple Checker for mmap calls
NoQ added a comment.
All right, so the code looks good now, but in order to commit this, we also
need tests. We've got those automatic tests of ours in `test/Analysis/` that
feed small code snippets into the analyzer and verify that it does (or doesn't)
emit warnings in certain cases, so you'd need to prove that your checker works
(as intended, ideally) by writing some of those
(http://clang-analyzer.llvm.org/checker_dev_manual.html#testing).
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:45-46
+ CheckerContext &C) const {
+ if (Call.isCalled(MmapFn)) {
+if (Call.getNumArgs() < 3)
+ return;
You can include the required number of arguments in CallDescription.
Repository:
rC Clang
https://reviews.llvm.org/D42645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132578.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,78 @@
+// 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") {}
+ 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)) {
+if (Call.getNumArgs() < 3)
+ return;
+
+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 "
+ "leads 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
MisusedMovedObjectChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -86,7 +86,7 @@
// The APIModeling package is for checkers that model APIs and don't perform
// any diagnostics. These checkers are always turned on; this package is
-// intended for API modeling that is not controlled by the target triple.
+// intended for API modeling that is not controlled by the the target triple.
def APIModeling : Package<"apiModeling">, Hidden;
def GoogleAPIModeling : Package<"google">, InPackage;
@@ -394,6 +394,10 @@
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 {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. Updated. I ve tried (with few personal code) called as f/ptr, prots set via variable as well. Might sounds obvious to you though this is my first contribution to this :-) Any chance it get pushed ? Just asking I test constantly/carry patches between 3 machines :-) Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132459.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,78 @@
+// 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") {}
+ 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)) {
+if (Call.getNumArgs() < 3)
+ return;
+
+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"));
+
+ 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 "
+ "leads 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
MisusedMovedObjectChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -86,7 +86,7 @@
// The APIModeling package is for checkers that model APIs and don't perform
// any diagnostics. These checkers are always turned on; this package is
-// intended for API modeling that is not controlled by the target triple.
+// intended for API modeling that is not controlled by the the target triple.
def APIModeling : Package<"apiModeling">, Hidden;
def GoogleAPIModeling : Package<"google">, InPackage;
@@ -394,6 +394,10 @@
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 {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:62 + + ExplodedNode *N = C.generateErrorNode(); + if (!N) You should also use `generateNonFatalErrorNode()` here so that not to prevent the analyzer from finding other bugs on that path, because your bug doesn't cause abnormal program termination or otherwise leave the analyzer in an inconsistent state. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. Another example with pcre2, more specifically its JIT engine In file included from /home/dcarlier/Contribs/php-src/ext/pcre/pcre2lib/pcre2_jit_compile.c:78: In file included from /home/dcarlier/Contribs/php-src/ext/pcre/pcre2lib/sljit/sljitLir.c:261: /home/dcarlier/Contribs/php-src/ext/pcre/pcre2lib/sljit/sljitExecAllocator.c:102:11: warning: Both PROT_WRITE and PROT_EXEC flags had been set. It can leads to exploitable memory regions, overwritten with malicious code retval = mmap(NULL, size, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0); ^~~ In file included from /home/dcarlier/Contribs/php-src/ext/pcre/pcre2lib/pcre2_jit_compile.c:78: In file included from /home/dcarlier/Contribs/php-src/ext/pcre/pcre2lib/sljit/sljitLir.c:1737: Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added a comment. In https://reviews.llvm.org/D42645#990771, @a.sidorin wrote: > Hello David, > > Do you have any results of this checker on the real code? If yes, could you > please share them? > There are also some inline comments regarding implementation. I did a quick test on the PHP opcache's code : warning: Both PROT_WRITE and PROT_EXEC flags had been set. It can leads to exploitable memory regions, overwritten with malicious code ret = mmap(start, size, PROT_READ | PROT_WRITE | PROT_EXEC, ^~~ ```~~ Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132054.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,78 @@
+// 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") {}
+ 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)) {
+if (Call.getNumArgs() < 3)
+ return;
+
+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"));
+
+ ExplodedNode *N = C.generateErrorNode();
+ if (!N)
+return;
+
+ auto Report = llvm::make_unique(
+ *BT, "Both PROT_WRITE and PROT_EXEC flags had been set. It can "
+ "leads 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
MisusedMovedObjectChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -86,7 +86,7 @@
// The APIModeling package is for checkers that model APIs and don't perform
// any diagnostics. These checkers are always turned on; this package is
-// intended for API modeling that is not controlled by the target triple.
+// intended for API modeling that is not controlled by the the target triple.
def APIModeling : Package<"apiModeling">, Hidden;
def GoogleAPIModeling : Package<"google">, InPackage;
@@ -394,6 +394,10 @@
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 {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
NoQ edited reviewers, added: NoQ; removed: dergachev.a. NoQ added a comment. Actually, most of our `security` package is off by default (in the driver) for that very reason, so please never mind and just keep it in `security` :) You can also put the checker straight into `security` bypassing `alpha` if you're not seeing any obvious problems with it - because it seems fairly straightforward and obvious. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
NoQ added a comment. > `// It's somehow an optional checker since for example in JIT libraries it is > pretty common.` Dunno, it seems that most of our security checks are something like that, and i'm not sure how to deal with it. This one sounds like it would probably go to some sort of `optin.security` (off-by-default). I also guess that there's no easy suppression, i.e. the user cannot easily change his code to suppress the warning in a specific place where it is intentional. In the analyzer we don't have many opt-in checks, because we focus more on users that want the analyzer to work as best as possible out of the box; for now we don't have a good interface for discovering optional checks (until an external entity makes the decision for the user while including the analyzer, eg. localization checks in Xcode), so most users would probably never see them. Also, do you need path sensitivity here? Like, if it is not common to pass the flags around in variables, or or call `mmap` by function pointer, then you'd have equally easy time catching most of these bugs with a simple syntax check, maybe a clang-tidy matcher. Hint - people like it when patches have large contexts (`git diff -U9` or something like that), they'd be clickable in the review interface. Repository: rC Clang https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 132024.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,78 @@
+// 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") {}
+ 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)) {
+if (Call.getNumArgs() < 3)
+ return;
+
+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"));
+
+ ExplodedNode *N = C.generateErrorNode();
+ if (!N)
+return;
+
+ auto Report = llvm::make_unique(
+ *BT, "Both PROT_WRITE and PROT_EXEC flags had been set. It can "
+ "leads 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();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocChecker.cpp
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
+ MmapWriteExecChecker.cpp
MisusedMovedObjectChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -86,7 +86,7 @@
// The APIModeling package is for checkers that model APIs and don't perform
// any diagnostics. These checkers are always turned on; this package is
-// intended for API modeling that is not controlled by the target triple.
+// intended for API modeling that is not controlled by the the target triple.
def APIModeling : Package<"apiModeling">, Hidden;
def GoogleAPIModeling : Package<"google">, InPackage;
@@ -414,6 +414,10 @@
HelpText<"Check for overflows in the arguments to malloc()">,
DescFile<"MallocOverflowSecurityChecker.cpp">;
+def MmapWriteExecChecker : Checker<"MmapWriteExec">,
+ HelpText<"Check if mmap() call is not both writable and executable">,
+ DescFile<"MmapWriteExecChecker.cpp">;
+
} // end "alpha.security"
//===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
a.sidorin added a comment.
Hello David,
Do you have any results of this checker on the real code? If yes, could you
please share them?
There are also some inline comments regarding implementation.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:42
+
+void MmapWriteExecChecker::checkPostStmt(const CallExpr *CE,
+ CheckerContext &C) const {
For analysis of function arguments, PreCall callback is more suitable because
it is called earlier.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:49
+ if (FName == "mmap") {
+SVal ProtVal = C.getSVal(CE->getArg(2));
+Optional Prot = ProtVal.getAs();
We need to check that the function call has at least three arguments to avoid
crashing on weird redeclarations like `void mmap()`.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:51
+Optional Prot = ProtVal.getAs();
+int64_t prot = Prot->getValue().getSExtValue();
+
Please follow LLVM naming conventions: variable names should start with capital.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:54
+if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
+ if (!BT) {
+BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags
set"));
Looks like this check is written in the way that allows to emit warning only
once. Did you mean:
```
if (!BT)
BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags );
ExplodedNode *N = C.generateErrorNode();
if (!N)
return;
...
```
?
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:55
+ if (!BT) {
+BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags
set"));
+ExplodedNode *N = C.generateErrorNode();
Could you please give more informative warning message describing the situation
and why is it harmful? Note that if user has more information, he can fix the
problem faster.
Repository:
rC Clang
https://reviews.llvm.org/D42645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
jroelofs added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:1
+// MmapWriteExecChecker.cpp - Check for the prot argument
+//
jroelofs wrote:
> Needs one of these at the top:
>
> ```
> //===- MmapWriteExecChecker.cpp - Check the mmap prot argument
> ---*- C++ -*-===//
> ```
>
> Appropriately fitted to 80-cols.
The format of the first line is important because some people's syntax
highlighters rely on it.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:29
+class MmapWriteExecChecker : public Checker> {
+#ifdef __GLIBC__
+ static const int ProtWrite = 0x02;
This makes the checker's behavior depend on whether the host was built against
glibc, however the target might not be the same as the host. You'll need to
check the target's defines at checker-run-time, and not hardcode this.
Repository:
rC Clang
https://reviews.llvm.org/D42645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:10
+//
+// This checker detects a common memory allocation security flaw.
+// Suppose 'unsigned int n' comes from an untrusted source. If the
jroelofs wrote:
> This comment was lifted from `MallocOverflowSecurityChecker.cpp`, and doesn't
> accurately describe what *this* checker does.
Exact sorry for that I created this patch in another machine and forgot to
update. For sure I used MallocOverflowSecurityChecker header as template.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:58
+
+if ((prot & (ProtWrite | ProtExec))) {
+ if (!BT) {
jroelofs wrote:
> I assume you meant:
>
>
> ```
> if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
> ```
>
> ?
True
Repository:
rC Clang
https://reviews.llvm.org/D42645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen updated this revision to Diff 131813.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,71 @@
+// 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/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+using llvm::APSInt;
+
+namespace {
+class MmapWriteExecChecker : public Checker> {
+#ifdef __GLIBC__
+ static const int ProtWrite = 0x02;
+ static const int ProtExec = 0x01;
+#else
+ static const int ProtWrite = 0x02;
+ static const int ProtExec = 0x04;
+#endif
+ mutable std::unique_ptr BT;
+public:
+ void checkPostStmt(const CallExpr *S, CheckerContext &C) const;
+};
+}
+
+void MmapWriteExecChecker::checkPostStmt(const CallExpr *CE,
+ CheckerContext &C) const {
+ StringRef FName = C.getCalleeName(CE);
+ if (FName.empty())
+return;
+
+ if (FName == "mmap") {
+SVal ProtVal = C.getSVal(CE->getArg(2));
+Optional Prot = ProtVal.getAs();
+int64_t prot = Prot->getValue().getSExtValue();
+
+if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
+ if (!BT) {
+BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags set"));
+ExplodedNode *N = C.generateErrorNode();
+if (!N)
+ return;
+
+auto Report = llvm::make_unique(
+ *BT, "Both PROT_WRITE and PROT_EXEC flags had been set", N);
+Report->addRange(CE->getArg(2)->getSourceRange());
+C.emitReport(std::move(Report));
+ }
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -50,6 +50,7 @@
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
MisusedMovedObjectChecker.cpp
+ MmapWriteExecChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
MPI-Checker/MPIFunctionClassifier.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -86,7 +86,7 @@
// The APIModeling package is for checkers that model APIs and don't perform
// any diagnostics. These checkers are always turned on; this package is
-// intended for API modeling that is not controlled by the the target triple.
+// intended for API modeling that is not controlled by the target triple.
def APIModeling : Package<"apiModeling">, Hidden;
def GoogleAPIModeling : Package<"google">, InPackage;
@@ -414,6 +414,10 @@
HelpText<"Check for overflows in the arguments to malloc()">,
DescFile<"MallocOverflowSecurityChecker.cpp">;
+def MmapWriteExecChecker : Checker<"MmapWriteExecChecker">,
+ HelpText<"Check if mmap() call is not both writable and executable">,
+ DescFile<"MmapWriteExecChecker.cpp">;
+
} // end "alpha.security"
//===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
jroelofs added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:1
+// MmapWriteExecChecker.cpp - Check for the prot argument
+//
Needs one of these at the top:
```
//===- MmapWriteExecChecker.cpp - Check the mmap prot argument
---*- C++ -*-===//
```
Appropriately fitted to 80-cols.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:10
+//
+// This checker detects a common memory allocation security flaw.
+// Suppose 'unsigned int n' comes from an untrusted source. If the
This comment was lifted from `MallocOverflowSecurityChecker.cpp`, and doesn't
accurately describe what *this* checker does.
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:58
+
+if ((prot & (ProtWrite | ProtExec))) {
+ if (!BT) {
I assume you meant:
```
if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
```
?
Repository:
rC Clang
https://reviews.llvm.org/D42645
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42645: New simple Checker for mmap calls
devnexen created this revision.
devnexen added reviewers: dcoughlin, dergachev.a.
devnexen created this object with visibility "All Users".
Herald added subscribers: cfe-commits, hintonda, mgorny.
This new checker tests if both PROT_WRITE and PROT_EXEC have been set.
Repository:
rC Clang
https://reviews.llvm.org/D42645
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
Index: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
@@ -0,0 +1,76 @@
+// 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 detects a common memory allocation security flaw.
+// Suppose 'unsigned int n' comes from an untrusted source. If the
+// code looks like 'malloc (n * 4)', and an attacker can make 'n' be
+// say MAX_UINT/4+2, then instead of allocating the correct 'n' 4-byte
+// elements, this will actually allocate only two because of overflow.
+// Then when the rest of the program attempts to store values past the
+// second element, these values will actually overwrite other items in
+// the heap, probably allowing the attacker to execute arbitrary code.
+//
+//===--===//
+
+#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/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+using llvm::APSInt;
+
+namespace {
+class MmapWriteExecChecker : public Checker> {
+#ifdef __GLIBC__
+ static const int ProtWrite = 0x02;
+ static const int ProtExec = 0x01;
+#else
+ static const int ProtWrite = 0x02;
+ static const int ProtExec = 0x04;
+#endif
+ mutable std::unique_ptr BT;
+public:
+ void checkPostStmt(const CallExpr *S, CheckerContext &C) const;
+};
+}
+
+void MmapWriteExecChecker::checkPostStmt(const CallExpr *CE,
+ CheckerContext &C) const {
+ StringRef FName = C.getCalleeName(CE);
+ if (FName.empty())
+return;
+
+ if (FName == "mmap") {
+SVal ProtVal = C.getSVal(CE->getArg(2));
+Optional Prot = ProtVal.getAs();
+int64_t prot = Prot->getValue().getSExtValue();
+
+if ((prot & (ProtWrite | ProtExec))) {
+ if (!BT) {
+BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags set"));
+ExplodedNode *N = C.generateErrorNode();
+if (!N)
+ return;
+
+auto Report = llvm::make_unique(
+ *BT, "Both PROT_WRITE and PROT_EXEC flags had been set", N);
+Report->addRange(CE->getArg(2)->getSourceRange());
+C.emitReport(std::move(Report));
+ }
+}
+ }
+}
+
+void ento::registerMmapWriteExecChecker(CheckerManager &mgr) {
+ mgr.registerChecker();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -50,6 +50,7 @@
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
MisusedMovedObjectChecker.cpp
+ MmapWriteExecChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
MPI-Checker/MPIFunctionClassifier.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -86,7 +86,7 @@
// The APIModeling package is for checkers that model APIs and don't perform
// any diagnostics. These checkers are always turned on; this package is
-// intended for API modeling that is not controlled by the the target triple.
+// intended for API modeling that is not controlled by the target triple.
def APIModeling : Package<"apiModeling">, Hidden;
def GoogleAPIModeling : Package<"google">, InPackage;
@@ -414,6 +414,10 @@
HelpText<"Check for overflows in the arguments to malloc()">,
DescFile<"MallocOverflowSecurityChecker.cpp">;
+def MmapWriteExecChecker : Checker<"MmapWriteExecChecker">,
+ HelpText<"Check if mmap() call is not both writable and executable">,
+ DescFile<"MmapWriteExecChecker.cpp">;
+
} // end "alpha.security"
//===--===//
___
cfe-commits mailing l
