diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 510629d8a2d48..899622ae283b1 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -213,9 +213,8 @@ Check for undefined results of binary operators. core.VLASize (C) """""""""""""""" -Check for declarations of Variable Length Arrays of undefined or zero size. - - Check for declarations of VLA of undefined or zero size. +Check for declarations of Variable Length Arrays (VLA) of undefined, zero or negative +size. .. code-block:: c @@ -229,6 +228,28 @@ Check for declarations of Variable Length Arrays of undefined or zero size. int vla2[x]; // warn: zero size } + +The checker also gives warning if the `TaintPropagation` checker is switched on +and an unbound, attacker controlled (tainted) value is used to define +the size of the VLA. + +.. code-block:: c + + void taintedVLA(void) { + int x; + scanf("%d", &x); + int vla[x]; // Declared variable-length array (VLA) has tainted (attacker controlled) size, that can be 0 or negative + } + + void taintedVerfieidVLA(void) { + int x; + scanf("%d", &x); + if (x<1) + return; + int vla[x]; // no-warning. The analyzer can prove that x must be positive. + } + + .. _core-uninitialized-ArraySubscript: core.uninitialized.ArraySubscript (C) diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index d76fe49918690..87d255eeffc17 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -164,12 +164,6 @@ ProgramStateRef VLASizeChecker::checkVLAIndexSize(CheckerContext &C, if (SizeV.isUnknown()) return nullptr; - // Check if the size is tainted. - if (isTainted(State, SizeV)) { - reportTaintBug(SizeE, State, C, SizeV); - return nullptr; - } - // Check if the size is zero. DefinedSVal SizeD = SizeV.castAs(); @@ -192,10 +186,10 @@ ProgramStateRef VLASizeChecker::checkVLAIndexSize(CheckerContext &C, SVal LessThanZeroVal = SVB.evalBinOp(State, BO_LT, SizeD, Zero, SVB.getConditionType()); + ProgramStateRef StatePos, StateNeg; if (std::optional LessThanZeroDVal = LessThanZeroVal.getAs()) { ConstraintManager &CM = C.getConstraintManager(); - ProgramStateRef StatePos, StateNeg; std::tie(StateNeg, StatePos) = CM.assumeDual(State, *LessThanZeroDVal); if (StateNeg && !StatePos) { @@ -205,6 +199,12 @@ ProgramStateRef VLASizeChecker::checkVLAIndexSize(CheckerContext &C, State = StatePos; } + // Check if the size is tainted. + if ((StateNeg || StateZero) && isTainted(State, SizeV)) { + reportTaintBug(SizeE, State, C, SizeV); + return nullptr; + } + return State; } @@ -218,7 +218,7 @@ void VLASizeChecker::reportTaintBug(const Expr *SizeE, ProgramStateRef State, SmallString<256> buf; llvm::raw_svector_ostream os(buf); os << "Declared variable-length array (VLA) "; - os << "has tainted size"; + os << "has tainted (attacker controlled) size that can be 0 or negative"; auto report = std::make_unique(TaintBT, os.str(), N); report->addRange(SizeE->getSourceRange()); diff --git a/clang/test/Analysis/taint-diagnostic-visitor.c b/clang/test/Analysis/taint-diagnostic-visitor.c index a3fa1639bffee..020e9579ac535 100644 --- a/clang/test/Analysis/taint-diagnostic-visitor.c +++ b/clang/test/Analysis/taint-diagnostic-visitor.c @@ -46,8 +46,8 @@ void taintDiagnosticVLA(void) { scanf("%d", &x); // expected-note {{Value assigned to 'x'}} // expected-note@-1 {{Taint originated here}} // expected-note@-2 {{Taint propagated to the 2nd argument}} - int vla[x]; // expected-warning {{Declared variable-length array (VLA) has tainted size}} - // expected-note@-1 {{Declared variable-length array (VLA) has tainted size}} + int vla[x]; // expected-warning {{Declared variable-length array (VLA) has tainted}} + // expected-note@-1 {{Declared variable-length array (VLA) has tainted}} } diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c index 4ff474b2ed40d..e85b4106a5806 100644 --- a/clang/test/Analysis/taint-generic.c +++ b/clang/test/Analysis/taint-generic.c @@ -405,7 +405,16 @@ int testDivByZero(void) { void testTaintedVLASize(void) { int x; scanf("%d", &x); - int vla[x]; // expected-warning{{Declared variable-length array (VLA) has tainted size}} + int vla[x]; // expected-warning{{Declared variable-length array (VLA) has tainted (attacker controlled) size that can be 0 or negative}} +} + +// Tainted-sanitized VLAs. +void testTaintedSanitizedVLASize(void) { + int x; + scanf("%d", &x); + if (x<1) + return; + int vla[x]; // no-warning } int testTaintedAllocaMem() {