diff options
| -rw-r--r-- | lib/StaticAnalyzer/Checkers/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | lib/StaticAnalyzer/Checkers/Checkers.td | 4 | ||||
| -rw-r--r-- | lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp | 262 | ||||
| -rw-r--r-- | test/Analysis/malloc-overflow.c | 114 |
4 files changed, 381 insertions, 0 deletions
diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 5968f007df..33d2c31fa0 100644 --- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -34,6 +34,7 @@ add_clang_library(clangStaticAnalyzerCheckers MacOSKeychainAPIChecker.cpp MacOSXAPIChecker.cpp MallocChecker.cpp + MallocOverflowSecurityChecker.cpp NSAutoreleasePoolChecker.cpp NSErrorChecker.cpp NoReturnFunctionChecker.cpp diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index fee689fd6f..e060ea26da 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -222,6 +222,10 @@ def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">, 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" //===----------------------------------------------------------------------===// diff --git a/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp new file mode 100644 index 0000000000..08f4fb1e82 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp @@ -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>(); +} + diff --git a/test/Analysis/malloc-overflow.c b/test/Analysis/malloc-overflow.c new file mode 100644 index 0000000000..91bdc874f7 --- /dev/null +++ b/test/Analysis/malloc-overflow.c @@ -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}} +} + |