diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 522905a152541..60de28e31c4f7 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -263,6 +263,11 @@ Improvements Moved checkers ^^^^^^^^^^^^^^ +- After lots of improvements, the checker ``alpha.security.ArrayBoundV2`` is + renamed to ``security.ArrayBound``. As this checker is stable now, the old + checker ``alpha.security.ArrayBound`` (which was searching for the same kind + of bugs with an different, simpler and less accurate algorithm) is removed. + .. _release-notes-sanitizers: Sanitizers diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index e093b2d672a74..707067358fdfe 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -1332,10 +1332,69 @@ security Security related checkers. +.. _security-ArrayBound: + +security.ArrayBound (C, C++) +"""""""""""""""""""""""""""" +Report out of bounds access to memory that is before the start or after the end +of the accessed region (array, heap-allocated region, string literal etc.). +This usually means incorrect indexing, but the checker also detects access via +the operators ``*`` and ``->``. + +.. code-block:: c + + void test_underflow(int x) { + int buf[100][100]; + if (x < 0) + buf[0][x] = 1; // warn + } + + void test_overflow() { + int buf[100]; + int *p = buf + 100; + *p = 1; // warn + } + +If checkers like :ref:`unix-Malloc` or :ref:`cplusplus-NewDelete` are enabled +to model the behavior of ``malloc()``, ``operator new`` and similar +allocators), then this checker can also reports out of bounds access to +dynamically allocated memory: + +.. code-block:: cpp + + int *test_dynamic() { + int *mem = new int[100]; + mem[-1] = 42; // warn + return mem; + } + +In uncertain situations (when the checker can neither prove nor disprove that +overflow occurs), the checker assumes that the the index (more precisely, the +memory offeset) is within bounds. + +However, if :ref:`optin-taint-GenericTaint` is enabled and the index/offset is +tainted (i.e. it is influenced by an untrusted souce), then this checker +reports the potential out of bounds access: + +.. code-block:: c + + void test_with_tainted_index() { + char s[] = "abc"; + int x = getchar(); + char c = s[x]; // warn: potential out of bounds access with tainted index + } + +.. note:: + + This checker is an improved and renamed version of the checker that was + previously known as ``alpha.security.ArrayBoundV2``. The old checker + ``alpha.security.ArrayBound`` was removed when the (previously + "experimental") V2 variant became stable enough for regular use. + .. _security-cert-env-InvalidPtr: security.cert.env.InvalidPtr -"""""""""""""""""""""""""""""""""" +"""""""""""""""""""""""""""" Corresponds to SEI CERT Rules `ENV31-C `_ and `ENV34-C `_. @@ -3216,78 +3275,6 @@ Warns against using one vs. many plural pattern in code when generating localize alpha.security ^^^^^^^^^^^^^^ -.. _alpha-security-ArrayBound: - -alpha.security.ArrayBound (C) -""""""""""""""""""""""""""""" -Warn about buffer overflows (older checker). - -.. code-block:: c - - void test() { - char *s = ""; - char c = s[1]; // warn - } - - struct seven_words { - int c[7]; - }; - - void test() { - struct seven_words a, *p; - p = &a; - p[0] = a; - p[1] = a; - p[2] = a; // warn - } - - // note: requires unix.Malloc or - // alpha.unix.MallocWithAnnotations checks enabled. - void test() { - int *p = malloc(12); - p[3] = 4; // warn - } - - void test() { - char a[2]; - int *b = (int*)a; - b[1] = 3; // warn - } - -.. _alpha-security-ArrayBoundV2: - -alpha.security.ArrayBoundV2 (C) -""""""""""""""""""""""""""""""" -Warn about buffer overflows (newer checker). - -.. code-block:: c - - void test() { - char *s = ""; - char c = s[1]; // warn - } - - void test() { - int buf[100]; - int *p = buf; - p = p + 99; - p[1] = 1; // warn - } - - // note: compiler has internal check for this. - // Use -Wno-array-bounds to suppress compiler warning. - void test() { - int buf[100][100]; - buf[0][-1] = 1; // warn - } - - // note: requires optin.taint check turned on. - void test() { - char s[] = "abc"; - int x = getchar(); - char c = s[x]; // warn: index is tainted - } - .. _alpha-security-ReturnPtrRange: alpha.security.ReturnPtrRange (C) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 1361da46c3c81..9bf491eac1e0e 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -989,30 +989,41 @@ def decodeValueOfObjCType : Checker<"decodeValueOfObjCType">, let ParentPackage = Security in { -def FloatLoopCounter : Checker<"FloatLoopCounter">, - HelpText<"Warn on using a floating point value as a loop counter (CERT: " - "FLP30-C, FLP30-CPP)">, - Dependencies<[SecuritySyntaxChecker]>, - Documentation; - -def MmapWriteExecChecker : Checker<"MmapWriteExec">, - HelpText<"Warn on mmap() calls with both writable and executable access">, - Documentation; - -def PointerSubChecker : Checker<"PointerSub">, - HelpText<"Check for pointer subtractions on two pointers pointing to " - "different memory chunks">, - Documentation; - -def PutenvStackArray : Checker<"PutenvStackArray">, - HelpText<"Finds calls to the function 'putenv' which pass a pointer to " - "an automatic (stack-allocated) array as the argument.">, - Documentation; - -def SetgidSetuidOrderChecker : Checker<"SetgidSetuidOrder">, - HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and " - "'setuid(getuid())' (CERT: POS36-C)">, - Documentation; + def ArrayBoundChecker : Checker<"ArrayBound">, + HelpText<"Warn about out of bounds access to memory">, + Documentation; + + def FloatLoopCounter + : Checker<"FloatLoopCounter">, + HelpText< + "Warn on using a floating point value as a loop counter (CERT: " + "FLP30-C, FLP30-CPP)">, + Dependencies<[SecuritySyntaxChecker]>, + Documentation; + + def MmapWriteExecChecker + : Checker<"MmapWriteExec">, + HelpText< + "Warn on mmap() calls with both writable and executable access">, + Documentation; + + def PointerSubChecker + : Checker<"PointerSub">, + HelpText<"Check for pointer subtractions on two pointers pointing to " + "different memory chunks">, + Documentation; + + def PutenvStackArray + : Checker<"PutenvStackArray">, + HelpText<"Finds calls to the function 'putenv' which pass a pointer to " + "an automatic (stack-allocated) array as the argument.">, + Documentation; + + def SetgidSetuidOrderChecker + : Checker<"SetgidSetuidOrder">, + HelpText<"Warn on possible reversed order of 'setgid(getgid()))' and " + "'setuid(getuid())' (CERT: POS36-C)">, + Documentation; } // end "security" @@ -1035,14 +1046,6 @@ let ParentPackage = ENV in { let ParentPackage = SecurityAlpha in { -def ArrayBoundChecker : Checker<"ArrayBound">, - HelpText<"Warn about buffer overflows (older checker)">, - Documentation; - -def ArrayBoundCheckerV2 : Checker<"ArrayBoundV2">, - HelpText<"Warn about buffer overflows (newer checker)">, - Documentation; - def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">, HelpText<"Check for an out-of-bound pointer being returned to callers">, Documentation; diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp deleted file mode 100644 index c990ad138f890..0000000000000 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ /dev/null @@ -1,92 +0,0 @@ -//== ArrayBoundChecker.cpp ------------------------------*- C++ -*--==// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// -// This file defines ArrayBoundChecker, which is a path-sensitive check -// which looks for an out-of-bound array element access. -// -//===----------------------------------------------------------------------===// - -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.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" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" - -using namespace clang; -using namespace ento; - -namespace { -class ArrayBoundChecker : - public Checker { - const BugType BT{this, "Out-of-bound array access"}; - -public: - void checkLocation(SVal l, bool isLoad, const Stmt* S, - CheckerContext &C) const; -}; -} - -void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS, - CheckerContext &C) const { - // Check for out of bound array element access. - const MemRegion *R = l.getAsRegion(); - if (!R) - return; - - const ElementRegion *ER = dyn_cast(R); - if (!ER) - return; - - // Get the index of the accessed element. - DefinedOrUnknownSVal Idx = ER->getIndex().castAs(); - - // Zero index is always in bound, this also passes ElementRegions created for - // pointer casts. - if (Idx.isZeroConstant()) - return; - - ProgramStateRef state = C.getState(); - - // Get the size of the array. - DefinedOrUnknownSVal ElementCount = getDynamicElementCount( - state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType()); - - ProgramStateRef StInBound, StOutBound; - std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, ElementCount); - if (StOutBound && !StInBound) { - ExplodedNode *N = C.generateErrorNode(StOutBound); - if (!N) - return; - - // FIXME: It would be nice to eventually make this diagnostic more clear, - // e.g., by referencing the original declaration or by saying *why* this - // reference is outside the range. - - // Generate a report for this bug. - auto report = std::make_unique( - BT, "Access out-of-bound array element (buffer overflow)", N); - - report->addRange(LoadS->getSourceRange()); - C.emitReport(std::move(report)); - return; - } - - // Array bound check succeeded. From this point forward the array bound - // should always succeed. - C.addTransition(StInBound); -} - -void ento::registerArrayBoundChecker(CheckerManager &mgr) { - mgr.registerChecker(); -} - -bool ento::shouldRegisterArrayBoundChecker(const CheckerManager &mgr) { - return true; -} diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index 6422933c8828a..6f8d6dbd573f4 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -6,11 +6,17 @@ // //===----------------------------------------------------------------------===// // -// This file defines ArrayBoundCheckerV2, which is a path-sensitive check -// which looks for an out-of-bound array element access. +// This file defines security.ArrayBound, which is a path-sensitive checker +// that looks for out of bounds access of memory regions. // //===----------------------------------------------------------------------===// +// NOTE: The name of this file ends with "V2" because previously +// "ArrayBoundChecker.cpp" contained the implementation of another (older and +// simpler) checker that was called `alpha.security.ArrayBound`. +// TODO: Rename this file to "ArrayBoundChecker.cpp" when it won't be confused +// with that older file. + #include "clang/AST/CharUnits.h" #include "clang/AST/ParentMapContext.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" @@ -124,9 +130,9 @@ struct Messages { // callbacks, we'd need to duplicate the logic that evaluates these // expressions.) The `MemberExpr` callback would work as `PreStmt` but it's // defined as `PostStmt` for the sake of consistency with the other callbacks. -class ArrayBoundCheckerV2 : public Checker, - check::PostStmt, - check::PostStmt> { +class ArrayBoundChecker : public Checker, + check::PostStmt, + check::PostStmt> { BugType BT{this, "Out-of-bound access"}; BugType TaintBT{this, "Out-of-bound access", categories::TaintedData}; @@ -547,7 +553,7 @@ bool StateUpdateReporter::providesInformationAboutInteresting( return false; } -void ArrayBoundCheckerV2::performCheck(const Expr *E, CheckerContext &C) const { +void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const { const SVal Location = C.getSVal(E); // The header ctype.h (from e.g. glibc) implements the isXXXXX() macros as @@ -670,9 +676,9 @@ void ArrayBoundCheckerV2::performCheck(const Expr *E, CheckerContext &C) const { C.addTransition(State, SUR.createNoteTag(C)); } -void ArrayBoundCheckerV2::markPartsInteresting(PathSensitiveBugReport &BR, - ProgramStateRef ErrorState, - NonLoc Val, bool MarkTaint) { +void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR, + ProgramStateRef ErrorState, + NonLoc Val, bool MarkTaint) { if (SymbolRef Sym = Val.getAsSymbol()) { // If the offset is a symbolic value, iterate over its "parts" with // `SymExpr::symbols()` and mark each of them as interesting. @@ -693,10 +699,10 @@ void ArrayBoundCheckerV2::markPartsInteresting(PathSensitiveBugReport &BR, } } -void ArrayBoundCheckerV2::reportOOB(CheckerContext &C, - ProgramStateRef ErrorState, Messages Msgs, - NonLoc Offset, std::optional Extent, - bool IsTaintBug /*=false*/) const { +void ArrayBoundChecker::reportOOB(CheckerContext &C, ProgramStateRef ErrorState, + Messages Msgs, NonLoc Offset, + std::optional Extent, + bool IsTaintBug /*=false*/) const { ExplodedNode *ErrorNode = C.generateErrorNode(ErrorState); if (!ErrorNode) @@ -725,7 +731,7 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &C, C.emitReport(std::move(BR)); } -bool ArrayBoundCheckerV2::isFromCtypeMacro(const Stmt *S, ASTContext &ACtx) { +bool ArrayBoundChecker::isFromCtypeMacro(const Stmt *S, ASTContext &ACtx) { SourceLocation Loc = S->getBeginLoc(); if (!Loc.isMacroID()) return false; @@ -744,7 +750,7 @@ bool ArrayBoundCheckerV2::isFromCtypeMacro(const Stmt *S, ASTContext &ACtx) { (MacroName == "isupper") || (MacroName == "isxdigit")); } -bool ArrayBoundCheckerV2::isInAddressOf(const Stmt *S, ASTContext &ACtx) { +bool ArrayBoundChecker::isInAddressOf(const Stmt *S, ASTContext &ACtx) { ParentMapContext &ParentCtx = ACtx.getParentMapContext(); do { const DynTypedNodeList Parents = ParentCtx.getParents(*S); @@ -756,10 +762,10 @@ bool ArrayBoundCheckerV2::isInAddressOf(const Stmt *S, ASTContext &ACtx) { return UnaryOp && UnaryOp->getOpcode() == UO_AddrOf; } -bool ArrayBoundCheckerV2::isIdiomaticPastTheEndPtr(const Expr *E, - ProgramStateRef State, - NonLoc Offset, NonLoc Limit, - CheckerContext &C) { +bool ArrayBoundChecker::isIdiomaticPastTheEndPtr(const Expr *E, + ProgramStateRef State, + NonLoc Offset, NonLoc Limit, + CheckerContext &C) { if (isa(E) && isInAddressOf(E, C.getASTContext())) { auto [EqualsToThreshold, NotEqualToThreshold] = compareValueToThreshold( State, Offset, Limit, C.getSValBuilder(), /*CheckEquality=*/true); @@ -768,10 +774,10 @@ bool ArrayBoundCheckerV2::isIdiomaticPastTheEndPtr(const Expr *E, return false; } -void ento::registerArrayBoundCheckerV2(CheckerManager &mgr) { - mgr.registerChecker(); +void ento::registerArrayBoundChecker(CheckerManager &mgr) { + mgr.registerChecker(); } -bool ento::shouldRegisterArrayBoundCheckerV2(const CheckerManager &mgr) { +bool ento::shouldRegisterArrayBoundChecker(const CheckerManager &mgr) { return true; } diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index fcbe8b864b6e4..ccff5d0ac3b96 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -7,7 +7,6 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangStaticAnalyzerCheckers AnalysisOrderChecker.cpp AnalyzerStatsChecker.cpp - ArrayBoundChecker.cpp ArrayBoundCheckerV2.cpp BasicObjCFoundationChecks.cpp BitwiseShiftChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 1a14f38e34f0e..39dcaf02dbe25 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -547,8 +547,10 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C, } return State; } - -// FIXME: This was originally copied from ArrayBoundChecker.cpp. Refactor? +// FIXME: The root of this logic was copied from the old checker +// alpha.security.ArrayBound (which is removed within this commit). +// It should be refactored to use the different, more sophisticated bounds +// checking logic used by the new checker ``security.ArrayBound``. ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, ProgramStateRef state, AnyArgExpr Buffer, SVal Element, diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp index 2c5cd2cf7630f..c81bb632b83e9 100644 --- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -891,9 +891,8 @@ DefinedOrUnknownSVal MemRegionManager::getStaticSize(const MemRegion *MR, return Size; } - // FIXME: The following are being used in 'SimpleSValBuilder' and in - // 'ArrayBoundChecker::checkLocation' because there is no symbol to - // represent the regions more appropriately. + // FIXME: The following are being used in 'SimpleSValBuilder' because there + // is no symbol to represent the regions more appropriately. case MemRegion::BlockDataRegionKind: case MemRegion::BlockCodeRegionKind: case MemRegion::FunctionCodeRegionKind: diff --git a/clang/test/Analysis/index-type.c b/clang/test/Analysis/index-type.c index 997d45c1e5aba..818806c4aff3b 100644 --- a/clang/test/Analysis/index-type.c +++ b/clang/test/Analysis/index-type.c @@ -1,5 +1,5 @@ -// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.security.ArrayBoundV2 -Wno-implicit-function-declaration -verify %s -// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.security.ArrayBoundV2 -Wno-implicit-function-declaration -DM32 -verify %s +// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,security.ArrayBound -Wno-implicit-function-declaration -verify %s +// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,security.ArrayBound -Wno-implicit-function-declaration -DM32 -verify %s // expected-no-diagnostics #define UINT_MAX (~0u) diff --git a/clang/test/Analysis/misc-ps-region-store.m b/clang/test/Analysis/misc-ps-region-store.m index a882e7eb0dc90..bd8b3350ba6da 100644 --- a/clang/test/Analysis/misc-ps-region-store.m +++ b/clang/test/Analysis/misc-ps-region-store.m @@ -1,5 +1,5 @@ -// RUN: %clang_analyze_cc1 -triple i386-apple-darwin9 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,alpha.security.ArrayBound -verify -fblocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s -// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -DTEST_64 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,alpha.security.ArrayBound -verify -fblocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s +// RUN: %clang_analyze_cc1 -triple i386-apple-darwin9 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,security.ArrayBound -verify -fblocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s +// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -DTEST_64 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,security.ArrayBound -verify -fblocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s typedef long unsigned int size_t; void *memcpy(void *, const void *, size_t); @@ -928,7 +928,7 @@ void pr6288_pos(int z) { int *px[1]; int i; for (i = 0; i < z; i++) - px[i] = &x[i]; // expected-warning{{Access out-of-bound array element (buffer overflow)}} + px[i] = &x[i]; // expected-warning{{Out of bound access to memory after the end of 'px'}} *(px[0]) = 0; // expected-warning{{Dereference of undefined pointer value}} } @@ -976,12 +976,17 @@ - (void) rdar7817800_baz { } @end -// PR 6036 - This test case triggered a crash inside StoreManager::CastRegion because the size -// of 'unsigned long (*)[0]' is 0. +// PR 6036 - This test case triggered a crash inside StoreManager::CastRegion +// because the size of 'unsigned long (*)[0]' is 0. +// NOTE: This old crash was probably triggered via the old alpha checker +// `alpha.security.ArrayBound` (which was logic that's different from the +// current `security.ArrayBound`). Although that code was removed, it's worth +// to keep this testcase as a generic example of a zero-sized type. struct pr6036_a { int pr6036_b; }; struct pr6036_c; void u132monitk (struct pr6036_c *pr6036_d) { - (void) ((struct pr6036_a *) (unsigned long (*)[0]) ((char *) pr6036_d - 1))->pr6036_b; // expected-warning{{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption}} + (void) ((struct pr6036_a *) (unsigned long (*)[0]) ((char *) pr6036_d - 1))->pr6036_b; + // expected-warning@-1 {{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption}} } // ?-expressions used as a base of a member expression should be treated as an lvalue diff --git a/clang/test/Analysis/no-outofbounds.c b/clang/test/Analysis/no-outofbounds.c index 15155729067e9..c6219ae74ab42 100644 --- a/clang/test/Analysis/no-outofbounds.c +++ b/clang/test/Analysis/no-outofbounds.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,alpha.unix,alpha.security.ArrayBound -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,alpha.unix,security.ArrayBound -verify %s // expected-no-diagnostics //===----------------------------------------------------------------------===// diff --git a/clang/test/Analysis/array-bound-v2-constraint-check.c b/clang/test/Analysis/out-of-bounds-constraint-check.c similarity index 97% rename from clang/test/Analysis/array-bound-v2-constraint-check.c rename to clang/test/Analysis/out-of-bounds-constraint-check.c index 91f748e655ce2..df48c8c170713 100644 --- a/clang/test/Analysis/array-bound-v2-constraint-check.c +++ b/clang/test/Analysis/out-of-bounds-constraint-check.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,alpha.security.ArrayBoundV2,debug.ExprInspection \ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,security.ArrayBound,debug.ExprInspection \ // RUN: -analyzer-config eagerly-assume=false -verify %s void clang_analyzer_eval(int); diff --git a/clang/test/Analysis/out-of-bounds-diagnostics.c b/clang/test/Analysis/out-of-bounds-diagnostics.c index 65bc28f58276f..1db01251148e1 100644 --- a/clang/test/Analysis/out-of-bounds-diagnostics.c +++ b/clang/test/Analysis/out-of-bounds-diagnostics.c @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-output=text \ -// RUN: -analyzer-checker=core,alpha.security.ArrayBoundV2,unix.Malloc,optin.taint -verify %s +// RUN: -analyzer-checker=core,security.ArrayBound,unix.Malloc,optin.taint -verify %s int TenElements[10]; diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp index f541bdf810d79..4e5442422bff4 100644 --- a/clang/test/Analysis/out-of-bounds-new.cpp +++ b/clang/test/Analysis/out-of-bounds-new.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,alpha.security.ArrayBoundV2 -verify %s +// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,security.ArrayBound -verify %s // Tests doing an out-of-bounds access after the end of an array using: // - constant integer index diff --git a/clang/test/Analysis/out-of-bounds-notes.c b/clang/test/Analysis/out-of-bounds-notes.c index 391089b6a35d8..7256beb7e893e 100644 --- a/clang/test/Analysis/out-of-bounds-notes.c +++ b/clang/test/Analysis/out-of-bounds-notes.c @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-output=text \ -// RUN: -analyzer-checker=core,alpha.security.ArrayBoundV2,unix.Malloc,optin.taint -verify %s +// RUN: -analyzer-checker=core,security.ArrayBound,unix.Malloc,optin.taint -verify %s int TenElements[10]; diff --git a/clang/test/Analysis/out-of-bounds.c b/clang/test/Analysis/out-of-bounds.c index 1f771c2b3bd13..734a56602e2aa 100644 --- a/clang/test/Analysis/out-of-bounds.c +++ b/clang/test/Analysis/out-of-bounds.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,security.ArrayBound,debug.ExprInspection -verify %s void clang_analyzer_eval(int); diff --git a/clang/test/Analysis/outofbound-notwork.c b/clang/test/Analysis/outofbound-notwork.c index cf2239cee1301..1318c07bbf2a8 100644 --- a/clang/test/Analysis/outofbound-notwork.c +++ b/clang/test/Analysis/outofbound-notwork.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,alpha.security.ArrayBound -verify %s +// RUN: %clang_analyze_cc1 -Wno-array-bounds -analyzer-checker=core,security.ArrayBound -verify %s // XFAIL: * // Once we better handle modeling of sizes of VLAs, we can pull this back @@ -9,7 +9,7 @@ void sizeof_vla(int a) { char x[a]; int y[sizeof(x)]; y[4] = 4; // no-warning - y[5] = 5; // expected-warning{{out-of-bound}} + y[5] = 5; // expected-warning{{Out of bounds access}} } } @@ -18,7 +18,7 @@ void sizeof_vla_2(int a) { char x[a]; int y[sizeof(x) / sizeof(char)]; y[4] = 4; // no-warning - y[5] = 5; // expected-warning{{out-of-bound}} + y[5] = 5; // expected-warning{{Out of bounds access}} } } @@ -27,6 +27,6 @@ void sizeof_vla_3(int a) { char x[a]; int y[sizeof(*&*&*&x)]; y[4] = 4; // no-warning - y[5] = 5; // expected-warning{{out-of-bound}} + y[5] = 5; // expected-warning{{Out of bounds access}} } } diff --git a/clang/test/Analysis/outofbound.c b/clang/test/Analysis/outofbound.c index 009cf33f61309..d3d8ff2b2f0ed 100644 --- a/clang/test/Analysis/outofbound.c +++ b/clang/test/Analysis/outofbound.c @@ -1,7 +1,7 @@ // RUN: %clang_analyze_cc1 -Wno-array-bounds -verify %s \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=unix \ -// RUN: -analyzer-checker=alpha.security.ArrayBound \ +// RUN: -analyzer-checker=security.ArrayBound \ // RUN: -analyzer-config unix.DynamicMemoryModeling:Optimistic=true typedef __typeof(sizeof(int)) size_t; @@ -11,12 +11,12 @@ void *calloc(size_t, size_t); char f1(void) { char* s = "abcd"; char c = s[4]; // no-warning - return s[5] + c; // expected-warning{{Access out-of-bound array element (buffer overflow)}} + return s[5] + c; // expected-warning{{Out of bound access to memory after}} } void f2(void) { int *p = malloc(12); - p[3] = 4; // expected-warning{{Access out-of-bound array element (buffer overflow)}} + p[3] = 4; // expected-warning{{Out of bound access to memory after}} } struct three_words { @@ -31,7 +31,7 @@ void f3(void) { struct three_words a, *p; p = &a; p[0] = a; // no-warning - p[1] = a; // expected-warning{{Access out-of-bound array element (buffer overflow)}} + p[1] = a; // expected-warning{{Out of bound access to memory after}} } void f4(void) { @@ -39,31 +39,33 @@ void f4(void) { struct three_words a, *p = (struct three_words *)&c; p[0] = a; // no-warning p[1] = a; // no-warning - p[2] = a; // expected-warning{{Access out-of-bound array element (buffer overflow)}} + p[2] = a; // should warn + // FIXME: This is an overflow, but currently security.ArrayBound only checks + // that the _beginning_ of the accessed element is within bounds. } void f5(void) { char *p = calloc(2,2); p[3] = '.'; // no-warning - p[4] = '!'; // expected-warning{{out-of-bound}} + p[4] = '!'; // expected-warning{{Out of bound access}} } void f6(void) { char a[2]; int *b = (int*)a; - b[1] = 3; // expected-warning{{out-of-bound}} + b[1] = 3; // expected-warning{{Out of bound access}} } void f7(void) { struct three_words a; - a.c[3] = 1; // expected-warning{{out-of-bound}} + a.c[3] = 1; // expected-warning{{Out of bound access}} } void vla(int a) { if (a == 5) { int x[a]; x[4] = 4; // no-warning - x[5] = 5; // expected-warning{{out-of-bound}} + x[5] = 5; // expected-warning{{Out of bound access}} } } @@ -71,14 +73,14 @@ void alloca_region(int a) { if (a == 5) { char *x = __builtin_alloca(a); x[4] = 4; // no-warning - x[5] = 5; // expected-warning{{out-of-bound}} + x[5] = 5; // expected-warning{{Out of bound access}} } } int symbolic_index(int a) { int x[2] = {1, 2}; if (a == 2) { - return x[a]; // expected-warning{{out-of-bound}} + return x[a]; // expected-warning{{Out of bound access}} } return 0; } @@ -86,7 +88,7 @@ int symbolic_index(int a) { int symbolic_index2(int a) { int x[2] = {1, 2}; if (a < 0) { - return x[a]; // expected-warning{{out-of-bound}} + return x[a]; // expected-warning{{Out of bound access}} } return 0; } @@ -120,7 +122,7 @@ int overflow_binary_search(double in) { } else { eee += 1; } - if (in < ins[eee]) { // expected-warning {{Access out-of-bound array element (buffer overflow)}} + if (in < ins[eee]) { // expected-warning {{Out of bound access}} eee -= 1; } } diff --git a/clang/test/Analysis/rdar-6541136-region.c b/clang/test/Analysis/rdar-6541136-region.c deleted file mode 100644 index f1a3a48a5fe4a..0000000000000 --- a/clang/test/Analysis/rdar-6541136-region.c +++ /dev/null @@ -1,27 +0,0 @@ -// RUN: %clang_analyze_cc1 -verify -analyzer-checker=core,alpha.security.ArrayBound %s - -struct tea_cheese { unsigned magic; }; -typedef struct tea_cheese kernel_tea_cheese_t; -extern kernel_tea_cheese_t _wonky_gesticulate_cheese; - -// This test case exercises the ElementRegion::getRValueType() logic. - -void test1( void ) { - kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese; - struct load_wine *cmd = (void*) &wonky[1]; - cmd = cmd; - char *p = (void*) &wonky[1]; - kernel_tea_cheese_t *q = &wonky[1]; - // This test case tests both the RegionStore logic (doesn't crash) and - // the out-of-bounds checking. We don't expect the warning for now since - // out-of-bound checking is temporarily disabled. - kernel_tea_cheese_t r = *q; // expected-warning{{Access out-of-bound array element (buffer overflow)}} -} - -void test1_b( void ) { - kernel_tea_cheese_t *wonky = &_wonky_gesticulate_cheese; - struct load_wine *cmd = (void*) &wonky[1]; - cmd = cmd; - char *p = (void*) &wonky[1]; - *p = 1; // expected-warning{{Access out-of-bound array element (buffer overflow)}} -} diff --git a/clang/test/Analysis/runtime-regression.c b/clang/test/Analysis/runtime-regression.c index 55988e9df5ee0..9fa0017d5e470 100644 --- a/clang/test/Analysis/runtime-regression.c +++ b/clang/test/Analysis/runtime-regression.c @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 %s \ -// RUN: -analyzer-checker=core,alpha.security.ArrayBoundV2 \ +// RUN: -analyzer-checker=core,security.ArrayBound \ // RUN: -analyzer-checker=debug.ExprInspection \ // RUN: -triple x86_64-unknown-linux-gnu \ // RUN: -verify diff --git a/clang/test/Analysis/taint-diagnostic-visitor.c b/clang/test/Analysis/taint-diagnostic-visitor.c index 223df9951fd6b..afe7117db7150 100644 --- a/clang/test/Analysis/taint-diagnostic-visitor.c +++ b/clang/test/Analysis/taint-diagnostic-visitor.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=optin.taint,core,alpha.security.ArrayBoundV2 -analyzer-output=text -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=optin.taint,core,security.ArrayBound -analyzer-output=text -verify %s // This file is for testing enhanced diagnostics produced by the GenericTaintChecker diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c index ad5a99fe8b3a3..3c520612c5d9b 100644 --- a/clang/test/Analysis/taint-generic.c +++ b/clang/test/Analysis/taint-generic.c @@ -3,7 +3,7 @@ // RUN: -analyzer-checker=optin.taint.GenericTaint \ // RUN: -analyzer-checker=optin.taint.TaintedDiv \ // RUN: -analyzer-checker=core \ -// RUN: -analyzer-checker=alpha.security.ArrayBoundV2 \ +// RUN: -analyzer-checker=security.ArrayBound \ // RUN: -analyzer-checker=debug.ExprInspection \ // RUN: -analyzer-config \ // RUN: optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml @@ -14,7 +14,7 @@ // RUN: -analyzer-checker=optin.taint.GenericTaint \ // RUN: -analyzer-checker=optin.taint.TaintedDiv \ // RUN: -analyzer-checker=core \ -// RUN: -analyzer-checker=alpha.security.ArrayBoundV2 \ +// RUN: -analyzer-checker=security.ArrayBound \ // RUN: -analyzer-checker=debug.ExprInspection \ // RUN: -analyzer-config \ // RUN: optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml diff --git a/clang/test/Analysis/taint-generic.cpp b/clang/test/Analysis/taint-generic.cpp index 881c5baf889f6..8836e1d3d2d98 100644 --- a/clang/test/Analysis/taint-generic.cpp +++ b/clang/test/Analysis/taint-generic.cpp @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -std=c++11 -Wno-format-security \ -// RUN: -analyzer-checker=core,optin.taint,alpha.security.ArrayBoundV2,debug.ExprInspection \ +// RUN: -analyzer-checker=core,optin.taint,security.ArrayBound,debug.ExprInspection \ // RUN: -analyzer-config optin.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml \ // RUN: -verify %s diff --git a/clang/www/analyzer/open_projects.html b/clang/www/analyzer/open_projects.html index a7c99c6e25bb3..4d6e40f92f82c 100644 --- a/clang/www/analyzer/open_projects.html +++ b/clang/www/analyzer/open_projects.html @@ -35,19 +35,6 @@

Open Projects

Such checkers should either be improved up to a point where they can be enabled by default, or removed from the analyzer entirely. - -
    -
  • alpha.security.ArrayBound and - alpha.security.ArrayBoundV2 -

    Array bounds checking is a desired feature, - but having an acceptable rate of false positives might not be possible - without a proper - loop widening support. - Additionally, it might be more promising to perform index checking based on - tainted index values. -

    (Difficulty: Medium)

    -
    • -
  • Improve C++ support diff --git a/clang/www/analyzer/potential_checkers.html b/clang/www/analyzer/potential_checkers.html index ad789b83e71b7..2543db251be12 100644 --- a/clang/www/analyzer/potential_checkers.html +++ b/clang/www/analyzer/potential_checkers.html @@ -965,7 +965,7 @@

    undefined behavior

    (C++03)
    Undefined behavior: out-of-bound basic_string access/modification.
    Note: possibly an enhancement to -alpha.security.ArrayBoundV2. +security.ArrayBound.

    Source: C++03 21.3.4p1; C++11 behavior is defined (21.4.5p2).