[cfe-commits] r136804 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp test/Analysis/malloc-overflow.c
Ted Kremenek
kremenek at apple.com
Wed Aug 3 13:17:43 PDT 2011
Author: kremenek
Date: Wed Aug 3 15:17:43 2011
New Revision: 136804
URL: http://llvm.org/viewvc/llvm-project?rev=136804&view=rev
Log:
[analyzer] Introduce MallocOverflowSecurityChecker, a simple flow-sensitive checker that may be useful for security auditing. This checker is currently too noisy to be on by default.
Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
cfe/trunk/test/Analysis/malloc-overflow.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=136804&r1=136803&r2=136804&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Wed Aug 3 15:17:43 2011
@@ -34,6 +34,7 @@
MacOSKeychainAPIChecker.cpp
MacOSXAPIChecker.cpp
MallocChecker.cpp
+ MallocOverflowSecurityChecker.cpp
NSAutoreleasePoolChecker.cpp
NSErrorChecker.cpp
NoReturnFunctionChecker.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=136804&r1=136803&r2=136804&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Wed Aug 3 15:17:43 2011
@@ -222,6 +222,10 @@
HelpText<"Check for an out-of-bound pointer being returned to callers">,
DescFile<"ReturnPointerRangeChecker.cpp">;
+def MallocOverflowSecurityChecker : Checker<"MallocOverflow">,
+ HelpText<"Check for overflows in the arguments to malloc()">,
+ DescFile<"MallocOverflowSecurityChecker.cpp">;
+
} // end "security.experimental"
//===----------------------------------------------------------------------===//
Added: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp?rev=136804&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp Wed Aug 3 15:17:43 2011
@@ -0,0 +1,262 @@
+// MallocOverflowSecurityChecker.cpp - Check for malloc overflows -*- C++ -*-=//
+//
+// 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/AST/EvaluatedExprVisitor.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "llvm/ADT/SmallVector.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+struct MallocOverflowCheck {
+ const BinaryOperator *mulop;
+ const Expr *variable;
+
+ MallocOverflowCheck (const BinaryOperator *m, const Expr *v)
+ : mulop(m), variable (v)
+ {}
+};
+
+class MallocOverflowSecurityChecker : public Checker<check::ASTCodeBody> {
+public:
+ void checkASTCodeBody(const Decl *D, AnalysisManager &mgr,
+ BugReporter &BR) const;
+
+ void CheckMallocArgument(
+ llvm::SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
+ const Expr *TheArgument, ASTContext &Context) const;
+
+ void OutputPossibleOverflows(
+ llvm::SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
+ const Decl *D, BugReporter &BR, AnalysisManager &mgr) const;
+
+};
+} // end anonymous namespace
+
+void MallocOverflowSecurityChecker::CheckMallocArgument(
+ llvm::SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
+ const Expr *TheArgument,
+ ASTContext &Context) const {
+
+ /* Look for a linear combination with a single variable, and at least
+ one multiplication.
+ Reject anything that applies to the variable: an explicit cast,
+ conditional expression, an operation that could reduce the range
+ of the result, or anything too complicated :-). */
+ const Expr * e = TheArgument;
+ const BinaryOperator * mulop = NULL;
+
+ for (;;) {
+ e = e->IgnoreParenImpCasts();
+ if (isa<BinaryOperator>(e)) {
+ const BinaryOperator * binop = dyn_cast<BinaryOperator>(e);
+ BinaryOperatorKind opc = binop->getOpcode();
+ // TODO: ignore multiplications by 1, reject if multiplied by 0.
+ if (mulop == NULL && opc == BO_Mul)
+ mulop = binop;
+ if (opc != BO_Mul && opc != BO_Add && opc != BO_Sub && opc != BO_Shl)
+ return;
+
+ const Expr *lhs = binop->getLHS();
+ const Expr *rhs = binop->getRHS();
+ if (rhs->isEvaluatable(Context))
+ e = lhs;
+ else if ((opc == BO_Add || opc == BO_Mul)
+ && lhs->isEvaluatable(Context))
+ e = rhs;
+ else
+ return;
+ }
+ else if (isa<DeclRefExpr>(e) || isa<MemberExpr>(e))
+ break;
+ else
+ return;
+ }
+
+ if (mulop == NULL)
+ return;
+
+ // We've found the right structure of malloc argument, now save
+ // the data so when the body of the function is completely available
+ // we can check for comparisons.
+
+ // TODO: Could push this into the innermost scope where 'e' is
+ // defined, rather than the whole function.
+ PossibleMallocOverflows.push_back(MallocOverflowCheck(mulop, e));
+}
+
+namespace {
+// A worker class for OutputPossibleOverflows.
+class CheckOverflowOps :
+ public EvaluatedExprVisitor<CheckOverflowOps> {
+public:
+ typedef llvm::SmallVectorImpl<MallocOverflowCheck> theVecType;
+
+private:
+ theVecType &toScanFor;
+ ASTContext &Context;
+
+ bool isIntZeroExpr(const Expr *E) const {
+ return (E->getType()->isIntegralOrEnumerationType()
+ && E->isEvaluatable(Context)
+ && E->EvaluateAsInt(Context) == 0);
+ }
+
+ void CheckExpr(const Expr *E_p) {
+ const Expr *E = E_p->IgnoreParenImpCasts();
+
+ theVecType::iterator i = toScanFor.end();
+ theVecType::iterator e = toScanFor.begin();
+
+ if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) {
+ const Decl * EdreD = DR->getDecl();
+ while (i != e) {
+ --i;
+ if (const DeclRefExpr *DR_i = dyn_cast<DeclRefExpr>(i->variable)) {
+ if (DR_i->getDecl() == EdreD)
+ i = toScanFor.erase(i);
+ }
+ }
+ }
+ else if (isa<MemberExpr>(E)) {
+ // No points-to analysis, just look at the member
+ const Decl * EmeMD = dyn_cast<MemberExpr>(E)->getMemberDecl();
+ while (i != e) {
+ --i;
+ if (isa<MemberExpr>(i->variable)) {
+ if (dyn_cast<MemberExpr>(i->variable)->getMemberDecl() == EmeMD)
+ i = toScanFor.erase (i);
+ }
+ }
+ }
+ }
+
+ public:
+ void VisitBinaryOperator(BinaryOperator *E) {
+ if (E->isComparisonOp()) {
+ const Expr * lhs = E->getLHS();
+ const Expr * rhs = E->getRHS();
+ // Ignore comparisons against zero, since they generally don't
+ // protect against an overflow.
+ if (!isIntZeroExpr(lhs) && ! isIntZeroExpr(rhs)) {
+ CheckExpr(lhs);
+ CheckExpr(rhs);
+ }
+ }
+ EvaluatedExprVisitor<CheckOverflowOps>::VisitBinaryOperator(E);
+ }
+
+ /* We specifically ignore loop conditions, because they're typically
+ not error checks. */
+ void VisitWhileStmt(WhileStmt *S) {
+ return this->Visit(S->getBody());
+ }
+ void VisitForStmt(ForStmt *S) {
+ return this->Visit(S->getBody());
+ }
+ void VisitDoStmt(DoStmt *S) {
+ return this->Visit(S->getBody());
+ }
+
+ CheckOverflowOps(theVecType &v, ASTContext &ctx)
+ : EvaluatedExprVisitor<CheckOverflowOps>(ctx),
+ toScanFor(v), Context(ctx)
+ { }
+ };
+}
+
+// OutputPossibleOverflows - We've found a possible overflow earlier,
+// now check whether Body might contain a comparison which might be
+// preventing the overflow.
+// This doesn't do flow analysis, range analysis, or points-to analysis; it's
+// just a dumb "is there a comparison" scan. The aim here is to
+// detect the most blatent cases of overflow and educate the
+// programmer.
+void MallocOverflowSecurityChecker::OutputPossibleOverflows(
+ llvm::SmallVectorImpl<MallocOverflowCheck> &PossibleMallocOverflows,
+ const Decl *D, BugReporter &BR, AnalysisManager &mgr) const {
+ // By far the most common case: nothing to check.
+ if (PossibleMallocOverflows.empty())
+ return;
+
+ // Delete any possible overflows which have a comparison.
+ CheckOverflowOps c(PossibleMallocOverflows, BR.getContext());
+ c.Visit(mgr.getAnalysisContext(D)->getBody());
+
+ // Output warnings for all overflows that are left.
+ for (CheckOverflowOps::theVecType::iterator
+ i = PossibleMallocOverflows.begin(),
+ e = PossibleMallocOverflows.end();
+ i != e;
+ ++i) {
+ SourceRange R = i->mulop->getSourceRange();
+ BR.EmitBasicReport("MallocOverflowSecurityChecker",
+ "the computation of the size of the memory allocation may overflow",
+ i->mulop->getOperatorLoc(), &R, 1);
+ }
+}
+
+void MallocOverflowSecurityChecker::checkASTCodeBody(const Decl *D,
+ AnalysisManager &mgr,
+ BugReporter &BR) const {
+
+ CFG *cfg = mgr.getCFG(D);
+ if (!cfg)
+ return;
+
+ // A list of variables referenced in possibly overflowing malloc operands.
+ llvm::SmallVector<MallocOverflowCheck, 2> PossibleMallocOverflows;
+
+ for (CFG::iterator it = cfg->begin(), ei = cfg->end(); it != ei; ++it) {
+ CFGBlock *block = *it;
+ for (CFGBlock::iterator bi = block->begin(), be = block->end();
+ bi != be; ++bi) {
+ if (const CFGStmt *CS = bi->getAs<CFGStmt>()) {
+ if (const CallExpr *TheCall = dyn_cast<CallExpr>(CS->getStmt())) {
+ // Get the callee.
+ const FunctionDecl *FD = TheCall->getDirectCallee();
+
+ if (!FD)
+ return;
+
+ // Get the name of the callee. If it's a builtin, strip off the prefix.
+ IdentifierInfo *FnInfo = FD->getIdentifier();
+
+ if (FnInfo->isStr ("malloc") || FnInfo->isStr ("_MALLOC")) {
+ if (TheCall->getNumArgs() == 1)
+ CheckMallocArgument(PossibleMallocOverflows, TheCall->getArg(0),
+ mgr.getASTContext());
+ }
+ }
+ }
+ }
+ }
+
+ OutputPossibleOverflows(PossibleMallocOverflows, D, BR, mgr);
+}
+
+void ento::registerMallocOverflowSecurityChecker(CheckerManager &mgr) {
+ mgr.registerChecker<MallocOverflowSecurityChecker>();
+}
+
Added: cfe/trunk/test/Analysis/malloc-overflow.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc-overflow.c?rev=136804&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/malloc-overflow.c (added)
+++ cfe/trunk/test/Analysis/malloc-overflow.c Wed Aug 3 15:17:43 2011
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 -analyze -analyzer-checker=security.experimental.MallocOverflow -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+extern void * malloc(size_t);
+
+void * f1(int n)
+{
+ return malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+}
+
+void * f2(int n)
+{
+ return malloc(sizeof(int) * n); // // expected-warning {{the computation of the size of the memory allocation may overflow}}
+}
+
+void * f3()
+{
+ return malloc(4 * sizeof(int)); // no-warning
+}
+
+struct s4
+{
+ int n;
+};
+
+void * f4(struct s4 *s)
+{
+ return malloc(s->n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+}
+
+void * f5(struct s4 *s)
+{
+ struct s4 s2 = *s;
+ return malloc(s2.n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+}
+
+void * f6(int n)
+{
+ return malloc((n + 1) * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+}
+
+#include <stddef.h>
+extern void * malloc (size_t);
+
+void * f7(int n)
+{
+ if (n > 10)
+ return NULL;
+ return malloc(n * sizeof(int)); // no-warning
+}
+
+void * f8(int n)
+{
+ if (n < 10)
+ return malloc(n * sizeof(int)); // no-warning
+ else
+ return NULL;
+}
+
+void * f9(int n)
+{
+ int * x = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+ for (int i = 0; i < n; i++)
+ x[i] = i;
+ return x;
+}
+
+void * f10(int n)
+{
+ int * x = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+ int i = 0;
+ while (i < n)
+ x[i++] = 0;
+ return x;
+}
+
+void * f11(int n)
+{
+ int * x = malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+ int i = 0;
+ do {
+ x[i++] = 0;
+ } while (i < n);
+ return x;
+}
+
+void * f12(int n)
+{
+ n = (n > 10 ? 10 : n);
+ int * x = malloc(n * sizeof(int)); // no-warning
+ for (int i = 0; i < n; i++)
+ x[i] = i;
+ return x;
+}
+
+struct s13
+{
+ int n;
+};
+
+void * f13(struct s13 *s)
+{
+ if (s->n > 10)
+ return NULL;
+ return malloc(s->n * sizeof(int)); // no warning
+}
+
+void * f14(int n)
+{
+ if (n < 0)
+ return NULL;
+ return malloc(n * sizeof(int)); // expected-warning {{the computation of the size of the memory allocation may overflow}}
+}
+
More information about the cfe-commits
mailing list