diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index 041e74e764fe..2c7dcd47a9e7 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -28,7 +28,7 @@ class ArrayBoundCheckerV2 : public Checker { mutable llvm::OwningPtr BT; - enum OOB_Kind { OOB_Precedes, OOB_Excedes }; + enum OOB_Kind { OOB_Precedes, OOB_Excedes, OOB_Tainted }; void reportOOB(CheckerContext &C, const ProgramState *errorState, OOB_Kind kind) const; @@ -157,7 +157,7 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad, // If we are under constrained and the index variables are tainted, report. if (state_exceedsUpperBound && state_withinUpperBound) { if (state->isTainted(rawOffset.getByteOffset())) - reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes); + reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted); return; } @@ -193,9 +193,18 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext, llvm::SmallString<256> buf; llvm::raw_svector_ostream os(buf); - os << "Out of bound memory access " - << (kind == OOB_Precedes ? "(accessed memory precedes memory block)" - : "(access exceeds upper limit of memory block)"); + os << "Out of bound memory access "; + switch (kind) { + case OOB_Precedes: + os << "(accessed memory precedes memory block)"; + break; + case OOB_Excedes: + os << "(access exceeds upper limit of memory block)"; + break; + case OOB_Tainted: + os << "(index is tainted)"; + break; + } checkerContext.EmitReport(new BugReport(*BT, os.str(), errorNode)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index b9ed384e0aa8..9f2f5151bff8 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -37,10 +37,10 @@ void DivZeroChecker::reportBug(const char *Msg, CheckerContext &C) const { if (ExplodedNode *N = C.generateSink(StateZero)) { if (!BT) - BT.reset(new BuiltinBug(Msg)); + BT.reset(new BuiltinBug("Division by zero")); BugReport *R = - new BugReport(*BT, BT->getDescription(), N); + new BugReport(*BT, Msg, N); R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, bugreporter::GetDenomExpr(N))); diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index 0308bf5c11b6..74d4f24348d1 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -26,14 +26,52 @@ using namespace ento; namespace { class VLASizeChecker : public Checker< check::PreStmt > { - mutable llvm::OwningPtr BT_zero; - mutable llvm::OwningPtr BT_undef; - + mutable llvm::OwningPtr BT; + enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted }; + + void reportBug(VLASize_Kind Kind, + const Expr *SizeE, + const ProgramState *State, + CheckerContext &C) const; public: void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const; }; } // end anonymous namespace +void VLASizeChecker::reportBug(VLASize_Kind Kind, + const Expr *SizeE, + const ProgramState *State, + CheckerContext &C) const { + // Generate an error node. + ExplodedNode *N = C.generateSink(State); + if (!N) + return; + + if (!BT) + BT.reset(new BuiltinBug("Dangerous variable-length array (VLA) declaration")); + + llvm::SmallString<256> buf; + llvm::raw_svector_ostream os(buf); + os << "Declared variable-length array (VLA) "; + switch (Kind) { + case VLA_Garbage: + os << "uses a garbage value as its size"; + break; + case VLA_Zero: + os << "has zero size"; + break; + case VLA_Tainted: + os << "has tainted size"; + break; + } + + BugReport *report = new BugReport(*BT, os.str(), N); + report->addRange(SizeE->getSourceRange()); + report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, SizeE)); + C.EmitReport(report); + return; +} + void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { if (!DS->isSingleDecl()) return; @@ -53,20 +91,7 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { SVal sizeV = state->getSVal(SE, C.getLocationContext()); if (sizeV.isUndef()) { - // Generate an error node. - ExplodedNode *N = C.generateSink(); - if (!N) - return; - - if (!BT_undef) - BT_undef.reset(new BuiltinBug("Declared variable-length array (VLA) " - "uses a garbage value as its size")); - - BugReport *report = - new BugReport(*BT_undef, BT_undef->getName(), N); - report->addRange(SE->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, SE)); - C.EmitReport(report); + reportBug(VLA_Garbage, SE, state, C); return; } @@ -75,6 +100,12 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { if (sizeV.isUnknown()) return; + // Check if the size is tainted. + if (state->isTainted(sizeV)) { + reportBug(VLA_Tainted, SE, 0, C); + return; + } + // Check if the size is zero. DefinedSVal sizeD = cast(sizeV); @@ -82,16 +113,7 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { llvm::tie(stateNotZero, stateZero) = state->assume(sizeD); if (stateZero && !stateNotZero) { - ExplodedNode *N = C.generateSink(stateZero); - if (!BT_zero) - BT_zero.reset(new BuiltinBug("Declared variable-length array (VLA) has " - "zero size")); - - BugReport *report = - new BugReport(*BT_zero, BT_zero->getName(), N); - report->addRange(SE->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, SE)); - C.EmitReport(report); + reportBug(VLA_Zero, SE, stateZero, C); return; } diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c index be20f2c24f60..d52dcda5a1f5 100644 --- a/clang/test/Analysis/taint-generic.c +++ b/clang/test/Analysis/taint-generic.c @@ -70,7 +70,7 @@ void scanfArg() { void bufferGetchar(int x) { int m = getchar(); - Buffer[m] = 1; //expected-warning {{Out of bound memory access }} + Buffer[m] = 1; //expected-warning {{Out of bound memory access (index is tainted)}} } void testUncontrolledFormatString(char **p) { @@ -176,3 +176,10 @@ int testDivByZero() { scanf("%d", &x); return 5/x; // expected-warning {{Division by a tainted value, possibly zero}} } + +// Zero-sized VLAs. +void testTaintedVLASize() { + int x; + scanf("%d", &x); + int vla[x]; // expected-warning{{Declared variable-length array (VLA) has tainted size}} +}