gamesh411 created this revision.
gamesh411 added a reviewer: steakhal.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy,
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
gamesh411 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
[BugReporter] Transitive interestingness
[Malloc] Pass down a State and a Pred ExplodedNode in the MallocChecker
[BoundV2] ArrayBoundV2 checks if the extent is tainted
[BoundV2][Malloc] Place NoteTags when allocated an interesting tainted amount
of memory
[CString] Add ConsiderTaint checker option for CStringChecker
[CString] Consider tainted out-of-bound accesses
[TaintProp] Place NoteTags when propagating taint
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125225
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/string.c
clang/test/Analysis/taint-diagnostic-visitor.c
Index: clang/test/Analysis/taint-diagnostic-visitor.c
===================================================================
--- clang/test/Analysis/taint-diagnostic-visitor.c
+++ clang/test/Analysis/taint-diagnostic-visitor.c
@@ -1,28 +1,31 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-output=text -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,core,unix.Malloc,alpha.security.ArrayBoundV2 -analyzer-output=text -verify %s
// This file is for testing enhanced diagnostics produced by the GenericTaintChecker
int scanf(const char *restrict format, ...);
int system(const char *command);
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t size);
+void free(void *ptr);
void taintDiagnostic(void)
{
char buf[128];
- scanf("%s", buf); // expected-note {{Taint originated here}}
+ scanf("%s", buf); // expected-note {{Propagated taint to the 2nd parameter}}
system(buf); // expected-warning {{Untrusted data is passed to a system call}} // expected-note {{Untrusted data is passed to a system call (CERT/STR02-C. Sanitize data passed to complex subsystems)}}
}
int taintDiagnosticOutOfBound(void) {
int index;
int Array[] = {1, 2, 3, 4, 5};
- scanf("%d", &index); // expected-note {{Taint originated here}}
+ scanf("%d", &index); // expected-note {{Taint originated here}} expected-note {{Propagated taint to the 2nd parameter}}
return Array[index]; // expected-warning {{Out of bound memory access (index is tainted)}}
// expected-note@-1 {{Out of bound memory access (index is tainted)}}
}
int taintDiagnosticDivZero(int operand) {
scanf("%d", &operand); // expected-note {{Value assigned to 'operand'}}
- // expected-note@-1 {{Taint originated here}}
+ // expected-note@-1 {{Taint originated here}} expected-note@-1 {{Propagated taint to the 2nd parameter}}
return 10 / operand; // expected-warning {{Division by a tainted value, possibly zero}}
// expected-note@-1 {{Division by a tainted value, possibly zero}}
}
@@ -30,7 +33,23 @@
void taintDiagnosticVLA(void) {
int x;
scanf("%d", &x); // expected-note {{Value assigned to 'x'}}
- // expected-note@-1 {{Taint originated here}}
+ // expected-note@-1 {{Taint originated here}} expected-note@-1 {{Propagated taint to the 2nd parameter}}
int vla[x]; // expected-warning {{Declared variable-length array (VLA) has tainted size}}
// expected-note@-1 {{Declared variable-length array (VLA) has tainted size}}
}
+
+void taintDiagnosticMalloc(int conj) {
+ int x;
+ scanf("%d", &x); // expected-note {{Taint originated here}}
+ // expected-note@-1 2 {{Propagated taint to the 2nd parameter}} Once for malloc(tainted), once for BoundsV2.
+
+ int *p = (int *)malloc(x + conj); // Generic taint checker forbids tainted allocation.
+ // expected-warning@-1 {{Untrusted data is used to specify the buffer size}}
+ // expected-note@-2 {{Untrusted data is used to specify the buffer size}}
+ // expected-note@-3 {{Allocating tainted amount of memory}}
+
+ p[1] = 1; // BoundsV2 checker can not prove that the access is safe.
+ // expected-warning@-1 {{Out of bound memory access (index is tainted)}}
+ // expected-note@-2 {{Out of bound memory access (index is tainted)}}
+ free(p);
+}
Index: clang/test/Analysis/string.c
===================================================================
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -1,10 +1,13 @@
-// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference \
+// RUN: %clang_analyze_cc1 %s -Wno-null-dereference \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=unix.cstring \
// RUN: -analyzer-checker=unix.Malloc \
+// RUN: -analyzer-checker=alpha.security.taint \
// RUN: -analyzer-checker=alpha.unix.cstring \
// RUN: -analyzer-checker=debug.ExprInspection \
-// RUN: -analyzer-config eagerly-assume=false
+// RUN: -analyzer-config alpha.unix.cstring.OutOfBounds:ConsiderTaint=true \
+// RUN: -analyzer-config eagerly-assume=false \
+// RUN: -verify=expected,tainted,OOB-consider-tainted
//
// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference -DUSE_BUILTINS \
// RUN: -analyzer-checker=core \
@@ -22,7 +25,7 @@
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-config eagerly-assume=false
//
-// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference \
+// RUN: %clang_analyze_cc1 %s -Wno-null-dereference \
// RUN: -DUSE_BUILTINS -DVARIANT \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=alpha.security.taint \
@@ -30,7 +33,8 @@
// RUN: -analyzer-checker=unix.Malloc \
// RUN: -analyzer-checker=alpha.unix.cstring \
// RUN: -analyzer-checker=debug.ExprInspection \
-// RUN: -analyzer-config eagerly-assume=false
+// RUN: -analyzer-config eagerly-assume=false \
+// RUN: -verify=expected,tainted
//
// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference \
// RUN: -DSUPPRESS_OUT_OF_BOUND \
@@ -481,6 +485,23 @@
}
#endif
+void strcat_overflow_tainted_dst_extent(char *y) {
+ int n;
+ scanf("%d", &n);
+ char *p = malloc(n); // tainted-warning {{Untrusted data is used to specify the buffer size}}
+ p[0] = '\0';
+
+ if (strlen(y) == 3)
+ strcat(p, y); // OOB-consider-tainted-warning {{String concatenation function might overflows the destination buffer}}
+ free(p);
+}
+
+void strcat_overflow_tainted_src_content(char *y) {
+ char x[4] = "12";
+ scanf("%s100", y); // Assuming that y is at least 100 bytes long.
+ strcat(x, y); // FIXME: The extent is not tainted, but we should still emit a warning here.
+}
+
void strcat_no_overflow(char *y) {
char x[5] = "12";
if (strlen(y) == 2)
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -12,6 +12,7 @@
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
+// CHECK-NEXT: alpha.unix.cstring.OutOfBounds:ConsiderTaint = false
// CHECK-NEXT: apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries = false
// CHECK-NEXT: apiModeling.StdCLibraryFunctions:ModelPOSIX = false
// CHECK-NEXT: apply-fixits = false
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2261,6 +2261,12 @@
// How to handle multiple metadata for the same region?
if (const auto *meta = dyn_cast<SymbolMetadata>(sym))
markInteresting(meta->getRegion(), TKind);
+
+ auto SubSyms = llvm::make_range(sym->symbol_begin(), sym->symbol_end());
+ for (SymbolRef SubSym : SubSyms) {
+ if (SymbolData::classof(SubSym))
+ insertToInterestingnessMap(InterestingSymbols, SubSym, TKind);
+ }
}
void PathSensitiveBugReport::markNotInteresting(SymbolRef sym) {
@@ -2341,10 +2347,25 @@
return None;
// We don't currently consider metadata symbols to be interesting
// even if we know their region is interesting. Is that correct behavior?
- auto It = InterestingSymbols.find(sym);
- if (It == InterestingSymbols.end())
- return None;
- return It->getSecond();
+ auto TryToLookupTrackingKind =
+ [this](SymbolRef Sym) -> Optional<bugreporter::TrackingKind> {
+ auto It = InterestingSymbols.find(Sym);
+ if (It == InterestingSymbols.end())
+ return None;
+ return It->getSecond();
+ };
+
+ if (auto MaybeTK = TryToLookupTrackingKind(sym))
+ return MaybeTK;
+
+ auto SubSyms = llvm::make_range(sym->symbol_begin(), sym->symbol_end());
+ for (SymbolRef SubSym : SubSyms) {
+ if (SymbolData::classof(SubSym)) {
+ if (auto MaybeTK = TryToLookupTrackingKind(SubSym))
+ return MaybeTK;
+ }
+ }
+ return None;
}
Optional<bugreporter::TrackingKind>
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -60,6 +60,7 @@
#include "clang/Basic/TargetInfo.h"
#include "clang/Lex/Lexer.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
@@ -214,16 +215,24 @@
REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, SymbolRef, RefState)
+namespace {
+struct StateAndPred {
+ ProgramStateRef State;
+ ExplodedNode *Pred;
+};
+} // namespace
+
/// Check if the memory associated with this symbol was released.
static bool isReleased(SymbolRef Sym, CheckerContext &C);
/// Update the RefState to reflect the new memory allocation.
/// The optional \p RetVal parameter specifies the newly allocated pointer
/// value; if unspecified, the value of expression \p E is used.
-static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
- ProgramStateRef State,
- AllocationFamily Family,
- Optional<SVal> RetVal = None);
+static StateAndPred MallocUpdateRefState(CheckerContext &C, const Expr *E,
+ ProgramStateRef State,
+ ExplodedNode *Pred,
+ AllocationFamily Family,
+ Optional<SVal> RetVal = None);
//===----------------------------------------------------------------------===//
// The modeling of memory reallocation.
@@ -455,9 +464,9 @@
/// Process C++ operator new()'s allocation, which is the part of C++
/// new-expression that goes before the constructor.
LLVM_NODISCARD
- ProgramStateRef processNewAllocation(const CXXAllocatorCall &Call,
- CheckerContext &C,
- AllocationFamily Family) const;
+ StateAndPred processNewAllocation(const CXXAllocatorCall &Call,
+ CheckerContext &C,
+ AllocationFamily Family) const;
/// Perform a zero-allocation check.
///
@@ -490,9 +499,10 @@
/// \param [in] State The \c ProgramState right before allocation.
/// \returns The ProgramState right after allocation.
LLVM_NODISCARD
- ProgramStateRef MallocMemReturnsAttr(CheckerContext &C, const CallEvent &Call,
- const OwnershipAttr *Att,
- ProgramStateRef State) const;
+ StateAndPred MallocMemReturnsAttr(CheckerContext &C, const CallEvent &Call,
+ const OwnershipAttr *Att,
+ ProgramStateRef State,
+ ExplodedNode *Pred) const;
/// Models memory allocation.
///
@@ -504,10 +514,10 @@
/// \param [in] State The \c ProgramState right before allocation.
/// \returns The ProgramState right after allocation.
LLVM_NODISCARD
- static ProgramStateRef MallocMemAux(CheckerContext &C, const CallEvent &Call,
- const Expr *SizeEx, SVal Init,
- ProgramStateRef State,
- AllocationFamily Family);
+ static StateAndPred MallocMemAux(CheckerContext &C, const CallEvent &Call,
+ const Expr *SizeEx, SVal Init,
+ ProgramStateRef State, ExplodedNode *Pred,
+ AllocationFamily Family);
/// Models memory allocation.
///
@@ -519,17 +529,17 @@
/// \param [in] State The \c ProgramState right before allocation.
/// \returns The ProgramState right after allocation.
LLVM_NODISCARD
- static ProgramStateRef MallocMemAux(CheckerContext &C, const CallEvent &Call,
- SVal Size, SVal Init,
- ProgramStateRef State,
- AllocationFamily Family);
+ static StateAndPred MallocMemAux(CheckerContext &C, const CallEvent &Call,
+ SVal Size, SVal Init, ProgramStateRef State,
+ ExplodedNode *Pred, AllocationFamily Family);
// Check if this malloc() for special flags. At present that means M_ZERO or
// __GFP_ZERO (in which case, treat it like calloc).
LLVM_NODISCARD
- llvm::Optional<ProgramStateRef>
- performKernelMalloc(const CallEvent &Call, CheckerContext &C,
- const ProgramStateRef &State) const;
+ llvm::Optional<StateAndPred> performKernelMalloc(const CallEvent &Call,
+ CheckerContext &C,
+ ProgramStateRef State,
+ ExplodedNode *Pred) const;
/// Model functions with the ownership_takes and ownership_holds attributes.
///
@@ -619,10 +629,10 @@
/// has an '_n' suffix, such as g_realloc_n.
/// \returns The ProgramState right after reallocation.
LLVM_NODISCARD
- ProgramStateRef ReallocMemAux(CheckerContext &C, const CallEvent &Call,
- bool ShouldFreeOnFail, ProgramStateRef State,
- AllocationFamily Family,
- bool SuffixWithN = false) const;
+ StateAndPred ReallocMemAux(CheckerContext &C, const CallEvent &Call,
+ bool ShouldFreeOnFail, ProgramStateRef State,
+ ExplodedNode *Pred, AllocationFamily Family,
+ bool SuffixWithN = false) const;
/// Evaluates the buffer size that needs to be allocated.
///
@@ -639,8 +649,8 @@
/// \param [in] State The \c ProgramState right before reallocation.
/// \returns The ProgramState right after allocation.
LLVM_NODISCARD
- static ProgramStateRef CallocMem(CheckerContext &C, const CallEvent &Call,
- ProgramStateRef State);
+ static StateAndPred CallocMem(CheckerContext &C, const CallEvent &Call,
+ ProgramStateRef State, ExplodedNode *Pred);
/// See if deallocation happens in a suspicious context. If so, escape the
/// pointers that otherwise would have been deallocated and return true.
@@ -1138,9 +1148,10 @@
return Func && Func->hasAttr<OwnershipAttr>();
}
-llvm::Optional<ProgramStateRef>
+llvm::Optional<StateAndPred>
MallocChecker::performKernelMalloc(const CallEvent &Call, CheckerContext &C,
- const ProgramStateRef &State) const {
+ ProgramStateRef State,
+ ExplodedNode *Pred) const {
// 3-argument malloc(), as commonly used in {Free,Net,Open}BSD Kernels:
//
// void *malloc(unsigned long size, struct malloc_type *mtp, int flags);
@@ -1211,7 +1222,7 @@
// If M_ZERO is set, treat this like calloc (initialized).
if (TrueState && !FalseState) {
SVal ZeroVal = C.getSValBuilder().makeZeroVal(Ctx.CharTy);
- return MallocMemAux(C, Call, Call.getArgExpr(0), ZeroVal, TrueState,
+ return MallocMemAux(C, Call, Call.getArgExpr(0), ZeroVal, TrueState, Pred,
AF_Malloc);
}
@@ -1231,24 +1242,24 @@
void MallocChecker::checkBasicAlloc(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
- State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State,
- AF_Malloc);
- State = ProcessZeroAllocCheck(Call, 0, State);
- C.addTransition(State);
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
+ Pair = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), Pair.State,
+ Pair.Pred, AF_Malloc);
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkKernelMalloc(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
- llvm::Optional<ProgramStateRef> MaybeState =
- performKernelMalloc(Call, C, State);
- if (MaybeState.hasValue())
- State = MaybeState.getValue();
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
+ llvm::Optional<StateAndPred> MaybePair =
+ performKernelMalloc(Call, C, Pair.State, Pair.Pred);
+ if (MaybePair.hasValue())
+ Pair = MaybePair.getValue();
else
- State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State,
- AF_Malloc);
- C.addTransition(State);
+ Pair = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), Pair.State,
+ Pair.Pred, AF_Malloc);
+ C.addTransition(Pair.State, Pair.Pred);
}
static bool isStandardRealloc(const CallEvent &Call) {
@@ -1289,19 +1300,18 @@
// https://bugs.llvm.org/show_bug.cgi?id=46253
if (!isStandardRealloc(Call) && !isGRealloc(Call))
return;
- ProgramStateRef State = C.getState();
- State = ReallocMemAux(C, Call, ShouldFreeOnFail, State, AF_Malloc);
- State = ProcessZeroAllocCheck(Call, 1, State);
- C.addTransition(State);
+ StateAndPred Pair = ReallocMemAux(C, Call, ShouldFreeOnFail, C.getState(),
+ C.getPredecessor(), AF_Malloc);
+ Pair.State = ProcessZeroAllocCheck(Call, 1, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkCalloc(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
- State = CallocMem(C, Call, State);
- State = ProcessZeroAllocCheck(Call, 0, State);
- State = ProcessZeroAllocCheck(Call, 1, State);
- C.addTransition(State);
+ StateAndPred Pair = CallocMem(C, Call, C.getState(), C.getPredecessor());
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State);
+ Pair.State = ProcessZeroAllocCheck(Call, 1, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkFree(const CallEvent &Call, CheckerContext &C) const {
@@ -1316,33 +1326,32 @@
void MallocChecker::checkAlloca(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
- State = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), State,
- AF_Alloca);
- State = ProcessZeroAllocCheck(Call, 0, State);
- C.addTransition(State);
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
+ Pair = MallocMemAux(C, Call, Call.getArgExpr(0), UndefinedVal(), Pair.State,
+ Pair.Pred, AF_Alloca);
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkStrdup(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
if (!CE)
return;
- State = MallocUpdateRefState(C, CE, State, AF_Malloc);
- C.addTransition(State);
+ StateAndPred Pair =
+ MallocUpdateRefState(C, CE, C.getState(), C.getPredecessor(), AF_Malloc);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkIfNameIndex(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
// Should we model this differently? We can allocate a fixed number of
// elements with zeros in the last one.
- State =
- MallocMemAux(C, Call, UnknownVal(), UnknownVal(), State, AF_IfNameIndex);
-
- C.addTransition(State);
+ Pair = MallocMemAux(C, Call, UnknownVal(), UnknownVal(), Pair.State,
+ Pair.Pred, AF_IfNameIndex);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkIfFreeNameIndex(const CallEvent &Call,
@@ -1356,7 +1365,7 @@
void MallocChecker::checkCXXNewOrCXXDelete(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
bool IsKnownToBeAllocatedMemory = false;
const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
if (!CE)
@@ -1371,85 +1380,88 @@
const FunctionDecl *FD = C.getCalleeDecl(CE);
switch (FD->getOverloadedOperator()) {
case OO_New:
- State =
- MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State, AF_CXXNew);
- State = ProcessZeroAllocCheck(Call, 0, State);
+ Pair = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), Pair.State,
+ Pair.Pred, AF_CXXNew);
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State);
break;
case OO_Array_New:
- State = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State,
- AF_CXXNewArray);
- State = ProcessZeroAllocCheck(Call, 0, State);
+ Pair = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), Pair.State,
+ Pair.Pred, AF_CXXNewArray);
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State);
break;
case OO_Delete:
- State = FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocatedMemory,
- AF_CXXNew);
+ Pair.State = FreeMemAux(C, Call, Pair.State, 0, false,
+ IsKnownToBeAllocatedMemory, AF_CXXNew);
break;
case OO_Array_Delete:
- State = FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocatedMemory,
- AF_CXXNewArray);
+ Pair.State = FreeMemAux(C, Call, Pair.State, 0, false,
+ IsKnownToBeAllocatedMemory, AF_CXXNewArray);
break;
default:
llvm_unreachable("not a new/delete operator");
}
- C.addTransition(State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkGMalloc0(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
SValBuilder &svalBuilder = C.getSValBuilder();
SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);
- State = MallocMemAux(C, Call, Call.getArgExpr(0), zeroVal, State, AF_Malloc);
- State = ProcessZeroAllocCheck(Call, 0, State);
- C.addTransition(State);
+ Pair = MallocMemAux(C, Call, Call.getArgExpr(0), zeroVal, Pair.State,
+ Pair.Pred, AF_Malloc);
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkGMemdup(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
- State =
- MallocMemAux(C, Call, Call.getArgExpr(1), UnknownVal(), State, AF_Malloc);
- State = ProcessZeroAllocCheck(Call, 1, State);
- C.addTransition(State);
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
+ Pair = MallocMemAux(C, Call, Call.getArgExpr(1), UnknownVal(), Pair.State,
+ Pair.Pred, AF_Malloc);
+ Pair.State = ProcessZeroAllocCheck(Call, 1, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkGMallocN(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
SVal Init = UndefinedVal();
SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
- State = MallocMemAux(C, Call, TotalSize, Init, State, AF_Malloc);
- State = ProcessZeroAllocCheck(Call, 0, State);
- State = ProcessZeroAllocCheck(Call, 1, State);
- C.addTransition(State);
+ Pair =
+ MallocMemAux(C, Call, TotalSize, Init, Pair.State, Pair.Pred, AF_Malloc);
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State);
+ Pair.State = ProcessZeroAllocCheck(Call, 1, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkGMallocN0(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
SValBuilder &SB = C.getSValBuilder();
SVal Init = SB.makeZeroVal(SB.getContext().CharTy);
SVal TotalSize = evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
- State = MallocMemAux(C, Call, TotalSize, Init, State, AF_Malloc);
- State = ProcessZeroAllocCheck(Call, 0, State);
- State = ProcessZeroAllocCheck(Call, 1, State);
- C.addTransition(State);
+ Pair =
+ MallocMemAux(C, Call, TotalSize, Init, Pair.State, Pair.Pred, AF_Malloc);
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State);
+ Pair.State = ProcessZeroAllocCheck(Call, 1, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkReallocN(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
- State = ReallocMemAux(C, Call, /*ShouldFreeOnFail=*/false, State, AF_Malloc,
- /*SuffixWithN=*/true);
- State = ProcessZeroAllocCheck(Call, 1, State);
- State = ProcessZeroAllocCheck(Call, 2, State);
- C.addTransition(State);
+ StateAndPred Pair = ReallocMemAux(C, Call, /*ShouldFreeOnFail=*/false,
+ C.getState(), C.getPredecessor(), AF_Malloc,
+ /*SuffixWithN=*/true);
+ Pair.State = ProcessZeroAllocCheck(Call, 1, Pair.State);
+ Pair.State = ProcessZeroAllocCheck(Call, 2, Pair.State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkOwnershipAttr(const CallEvent &Call,
CheckerContext &C) const {
- ProgramStateRef State = C.getState();
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
if (!CE)
return;
@@ -1464,16 +1476,16 @@
for (const auto *I : FD->specific_attrs<OwnershipAttr>()) {
switch (I->getOwnKind()) {
case OwnershipAttr::Returns:
- State = MallocMemReturnsAttr(C, Call, I, State);
+ Pair = MallocMemReturnsAttr(C, Call, I, Pair.State, Pair.Pred);
break;
case OwnershipAttr::Takes:
case OwnershipAttr::Holds:
- State = FreeMemAttr(C, Call, I, State);
+ Pair.State = FreeMemAttr(C, Call, I, Pair.State);
break;
}
}
}
- C.addTransition(State);
+ C.addTransition(Pair.State, Pair.Pred);
}
void MallocChecker::checkPostCall(const CallEvent &Call,
@@ -1613,41 +1625,42 @@
return false;
}
-ProgramStateRef
+StateAndPred
MallocChecker::processNewAllocation(const CXXAllocatorCall &Call,
CheckerContext &C,
AllocationFamily Family) const {
if (!isStandardNewDelete(Call))
- return nullptr;
+ return {nullptr, C.getPredecessor()};
const CXXNewExpr *NE = Call.getOriginExpr();
const ParentMap &PM = C.getLocationContext()->getParentMap();
- ProgramStateRef State = C.getState();
+ StateAndPred Pair = {C.getState(), C.getPredecessor()};
// Non-trivial constructors have a chance to escape 'this', but marking all
// invocations of trivial constructors as escaped would cause too great of
// reduction of true positives, so let's just do that for constructors that
// have an argument of a pointer-to-record type.
if (!PM.isConsumedExpr(NE) && hasNonTrivialConstructorCall(NE))
- return State;
+ return Pair;
// The return value from operator new is bound to a specified initialization
// value (if any) and we don't want to loose this value. So we call
// MallocUpdateRefState() instead of MallocMemAux() which breaks the
// existing binding.
SVal Target = Call.getObjectUnderConstruction();
- State = MallocUpdateRefState(C, NE, State, Family, Target);
- State = ProcessZeroAllocCheck(Call, 0, State, Target);
- return State;
+
+ Pair = MallocUpdateRefState(C, NE, Pair.State, Pair.Pred, Family, Target);
+ Pair.State = ProcessZeroAllocCheck(Call, 0, Pair.State, Target);
+ return Pair;
}
void MallocChecker::checkNewAllocator(const CXXAllocatorCall &Call,
CheckerContext &C) const {
if (!C.wasInlined) {
- ProgramStateRef State = processNewAllocation(
+ StateAndPred Pair = processNewAllocation(
Call, C,
(Call.getOriginExpr()->isArray() ? AF_CXXNewArray : AF_CXXNew));
- C.addTransition(State);
+ C.addTransition(Pair.State, Pair.Pred);
}
}
@@ -1698,48 +1711,49 @@
C.addTransition(State);
}
-ProgramStateRef
-MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallEvent &Call,
- const OwnershipAttr *Att,
- ProgramStateRef State) const {
+StateAndPred MallocChecker::MallocMemReturnsAttr(CheckerContext &C,
+ const CallEvent &Call,
+ const OwnershipAttr *Att,
+ ProgramStateRef State,
+ ExplodedNode *Pred) const {
if (!State)
- return nullptr;
+ return {nullptr, Pred};
if (Att->getModule()->getName() != "malloc")
- return nullptr;
+ return {nullptr, Pred};
OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end();
if (I != E) {
return MallocMemAux(C, Call, Call.getArgExpr(I->getASTIndex()),
- UndefinedVal(), State, AF_Malloc);
+ UndefinedVal(), State, Pred, AF_Malloc);
}
- return MallocMemAux(C, Call, UnknownVal(), UndefinedVal(), State, AF_Malloc);
+ return MallocMemAux(C, Call, UnknownVal(), UndefinedVal(), State, Pred,
+ AF_Malloc);
}
-ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
- const CallEvent &Call,
- const Expr *SizeEx, SVal Init,
- ProgramStateRef State,
- AllocationFamily Family) {
+StateAndPred MallocChecker::MallocMemAux(
+ CheckerContext &C, const CallEvent &Call, const Expr *SizeEx, SVal Init,
+ ProgramStateRef State, ExplodedNode *Pred, AllocationFamily Family) {
if (!State)
- return nullptr;
+ return {nullptr, Pred};
assert(SizeEx);
- return MallocMemAux(C, Call, C.getSVal(SizeEx), Init, State, Family);
+ return MallocMemAux(C, Call, C.getSVal(SizeEx), Init, State, Pred, Family);
}
-ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
- const CallEvent &Call, SVal Size,
- SVal Init, ProgramStateRef State,
- AllocationFamily Family) {
+StateAndPred MallocChecker::MallocMemAux(CheckerContext &C,
+ const CallEvent &Call, SVal Size,
+ SVal Init, ProgramStateRef State,
+ ExplodedNode *Pred,
+ AllocationFamily Family) {
if (!State)
- return nullptr;
+ return {nullptr, Pred};
const Expr *CE = Call.getOriginExpr();
// We expect the malloc functions to return a pointer.
if (!Loc::isLocType(CE->getType()))
- return nullptr;
+ return {nullptr, Pred};
// Bind the return value to the symbolic value from the heap region.
// TODO: We could rewrite post visit to eval call; 'malloc' does not have
@@ -1757,16 +1771,26 @@
// Set the region's extent.
State = setDynamicExtent(State, RetVal.getAsRegion(),
Size.castAs<DefinedOrUnknownSVal>(), svalBuilder);
+ if (taint::isTainted(State, Size)) {
+ const NoteTag *Note =
+ C.getNoteTag([Size](PathSensitiveBugReport &BR) -> std::string {
+ if (BR.isInteresting(Size))
+ return "Allocating tainted amount of memory";
+ return "";
+ });
+ Pred = C.addTransition(State, Pred, Note);
+ }
- return MallocUpdateRefState(C, CE, State, Family);
+ return MallocUpdateRefState(C, CE, State, Pred, Family);
}
-static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
- ProgramStateRef State,
- AllocationFamily Family,
- Optional<SVal> RetVal) {
+static StateAndPred MallocUpdateRefState(CheckerContext &C, const Expr *E,
+ ProgramStateRef State,
+ ExplodedNode *Pred,
+ AllocationFamily Family,
+ Optional<SVal> RetVal) {
if (!State)
- return nullptr;
+ return {nullptr, Pred};
// Get the return value.
if (!RetVal)
@@ -1774,7 +1798,7 @@
// We expect the malloc functions to return a pointer.
if (!RetVal->getAs<Loc>())
- return nullptr;
+ return {nullptr, Pred};
SymbolRef Sym = RetVal->getAsLocSymbol();
// This is a return value of a function that was not inlined, such as malloc()
@@ -1782,7 +1806,8 @@
assert(Sym);
// Set the symbol's state to Allocated.
- return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
+ State = State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
+ return {State, Pred};
}
ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
@@ -2571,24 +2596,25 @@
}
}
-ProgramStateRef
+StateAndPred
MallocChecker::ReallocMemAux(CheckerContext &C, const CallEvent &Call,
bool ShouldFreeOnFail, ProgramStateRef State,
- AllocationFamily Family, bool SuffixWithN) const {
+ ExplodedNode *Pred, AllocationFamily Family,
+ bool SuffixWithN) const {
if (!State)
- return nullptr;
+ return {nullptr, Pred};
const CallExpr *CE = cast<CallExpr>(Call.getOriginExpr());
if (SuffixWithN && CE->getNumArgs() < 3)
- return nullptr;
+ return {nullptr, Pred};
else if (CE->getNumArgs() < 2)
- return nullptr;
+ return {nullptr, Pred};
const Expr *arg0Expr = CE->getArg(0);
SVal Arg0Val = C.getSVal(arg0Expr);
if (!Arg0Val.getAs<DefinedOrUnknownSVal>())
- return nullptr;
+ return {nullptr, Pred};
DefinedOrUnknownSVal arg0Val = Arg0Val.castAs<DefinedOrUnknownSVal>();
SValBuilder &svalBuilder = C.getSValBuilder();
@@ -2604,7 +2630,7 @@
if (SuffixWithN)
TotalSize = evalMulForBufferSize(C, Arg1, CE->getArg(2));
if (!TotalSize.getAs<DefinedOrUnknownSVal>())
- return nullptr;
+ return {nullptr, Pred};
// Compare the size argument to 0.
DefinedOrUnknownSVal SizeZero =
@@ -2624,13 +2650,12 @@
// If the ptr is NULL and the size is not 0, the call is equivalent to
// malloc(size).
if (PrtIsNull && !SizeIsZero) {
- ProgramStateRef stateMalloc = MallocMemAux(
- C, Call, TotalSize, UndefinedVal(), StatePtrIsNull, Family);
- return stateMalloc;
+ return MallocMemAux(C, Call, TotalSize, UndefinedVal(), StatePtrIsNull,
+ Pred, Family);
}
if (PrtIsNull && SizeIsZero)
- return State;
+ return {State, Pred};
assert(!PrtIsNull);
@@ -2644,16 +2669,16 @@
// any constrains on the output pointer.
if (ProgramStateRef stateFree = FreeMemAux(
C, Call, StateSizeIsZero, 0, false, IsKnownToBeAllocated, Family))
- return stateFree;
+ return {stateFree, Pred};
// Default behavior.
if (ProgramStateRef stateFree =
FreeMemAux(C, Call, State, 0, false, IsKnownToBeAllocated, Family)) {
- ProgramStateRef stateRealloc =
- MallocMemAux(C, Call, TotalSize, UnknownVal(), stateFree, Family);
- if (!stateRealloc)
- return nullptr;
+ StateAndPred ReallocState =
+ MallocMemAux(C, Call, TotalSize, UnknownVal(), stateFree, Pred, Family);
+ if (!ReallocState.State)
+ return ReallocState;
OwnershipAfterReallocKind Kind = OAR_ToBeFreedAfterFailure;
if (ShouldFreeOnFail)
@@ -2671,30 +2696,30 @@
// Record the info about the reallocated symbol so that we could properly
// process failed reallocation.
- stateRealloc = stateRealloc->set<ReallocPairs>(ToPtr,
- ReallocPair(FromPtr, Kind));
+ ReallocState.State = ReallocState.State->set<ReallocPairs>(
+ ToPtr, ReallocPair(FromPtr, Kind));
// The reallocated symbol should stay alive for as long as the new symbol.
C.getSymbolManager().addSymbolDependency(ToPtr, FromPtr);
- return stateRealloc;
+ return ReallocState;
}
- return nullptr;
+ return {nullptr, Pred};
}
-ProgramStateRef MallocChecker::CallocMem(CheckerContext &C,
- const CallEvent &Call,
- ProgramStateRef State) {
+StateAndPred MallocChecker::CallocMem(CheckerContext &C, const CallEvent &Call,
+ ProgramStateRef State,
+ ExplodedNode *Pred) {
if (!State)
- return nullptr;
+ return {nullptr, Pred};
if (Call.getNumArgs() < 2)
- return nullptr;
+ return {nullptr, Pred};
SValBuilder &svalBuilder = C.getSValBuilder();
SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);
SVal TotalSize =
evalMulForBufferSize(C, Call.getArgExpr(0), Call.getArgExpr(1));
- return MallocMemAux(C, Call, TotalSize, zeroVal, State, AF_Malloc);
+ return MallocMemAux(C, Call, TotalSize, zeroVal, State, Pred, AF_Malloc);
}
MallocChecker::LeakInfo MallocChecker::getAllocationSite(const ExplodedNode *N,
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -19,6 +19,7 @@
#include "clang/Basic/Builtins.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Checkers/Taint.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -26,6 +27,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/YAMLTraits.h"
#include <limits>
@@ -38,7 +40,7 @@
using namespace ento;
using namespace taint;
-using llvm::ImmutableSet;
+using llvm::ImmutableList;
namespace {
@@ -432,13 +434,31 @@
} // namespace yaml
} // namespace llvm
-/// A set which is used to pass information from call pre-visit instruction
-/// to the call post-visit. The values are signed integers, which are either
-/// ReturnValueIndex, or indexes of the pointer/reference argument, which
-/// points to data, which should be tainted on return.
+namespace {
+struct FunctionParameter {
+ SVal Value;
+ ArgIdxTy Index;
+ FunctionParameter(SVal Value, ArgIdxTy Index) : Value{Value}, Index{Index} {}
+
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ Value.Profile(ID);
+ ID.AddInteger(Index);
+ }
+};
+
+bool operator==(const FunctionParameter& lhs, const FunctionParameter& rhs) {
+ return lhs.Value == rhs.Value && lhs.Index == rhs.Index;
+}
+} // namespace
+
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ReturnsTainted, bool)
+
+/// A list which is used to pass information from call pre-visit program point
+/// to the call post-visit. Contains the symbolic value of the tainted
+/// pointer/reference parameter and its index.
REGISTER_MAP_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, const LocationContext *,
- ImmutableSet<ArgIdxTy>)
-REGISTER_SET_FACTORY_WITH_PROGRAMSTATE(ArgIdxFactory, ArgIdxTy)
+ ImmutableList<FunctionParameter>)
+REGISTER_LIST_FACTORY_WITH_PROGRAMSTATE(FunctionParamFactory, FunctionParameter)
void GenericTaintRuleParser::validateArgVector(const std::string &Option,
const ArgVecTy &Args) const {
@@ -783,40 +803,92 @@
// checkPostStmt.
ProgramStateRef State = C.getState();
const StackFrameContext *CurrentFrame = C.getStackFrame();
+ ExplodedNode *Pred = C.getPredecessor();
// Depending on what was tainted at pre-visit, we determined a set of
// arguments which should be tainted after the function returns. These are
// stored in the state as TaintArgsOnPostVisit set.
- TaintArgsOnPostVisitTy TaintArgsMap = State->get<TaintArgsOnPostVisit>();
- const ImmutableSet<ArgIdxTy> *TaintArgs = TaintArgsMap.lookup(CurrentFrame);
+ if (State->get<ReturnsTainted>()) {
+ State = State->set<ReturnsTainted>(false);
+ State = taint::addTaint(State, Call.getReturnValue());
+ const NoteTag *Note =
+ C.getNoteTag([RetVal = Call.getReturnValue()](
+ PathSensitiveBugReport &BR) -> std::string {
+ return BR.isInteresting(RetVal) ? "Returned tainted value" : "";
+ });
+ Pred = C.addTransition(State, Pred, Note);
+ }
+
+ TaintArgsOnPostVisitTy TaintArgsMap = State->get<TaintArgsOnPostVisit>();
+ const ImmutableList<FunctionParameter> *TaintArgs = TaintArgsMap.lookup(CurrentFrame);
if (!TaintArgs)
return;
assert(!TaintArgs->isEmpty());
- LLVM_DEBUG(for (ArgIdxTy I
+ LLVM_DEBUG(for (FunctionParameter FP
: *TaintArgs) {
llvm::dbgs() << "PostCall<";
Call.dump(llvm::dbgs());
- llvm::dbgs() << "> actually wants to taint arg index: " << I << '\n';
+ llvm::dbgs() << "> actually wants to taint arg index: " << FP.Index << '\n';
});
- for (ArgIdxTy ArgNum : *TaintArgs) {
+ struct Pack {
+ SVal PreValue;
+ SVal PostValue;
+ ArgIdxTy Index;
+ };
+ SmallVector<Pack, 6> ActualTaintedParams;
+
+ for (FunctionParameter Param : *TaintArgs) {
// Special handling for the tainted return value.
- if (ArgNum == ReturnValueIndex) {
+ if (Param.Index == ReturnValueIndex) {
State = addTaint(State, Call.getReturnValue());
continue;
}
// The arguments are pointer arguments. The data they are pointing at is
// tainted after the call.
- if (auto V = getPointeeOf(C, Call.getArgSVal(ArgNum)))
+ if (auto V = getPointeeOf(C, Call.getArgSVal(Param.Index))) {
State = addTaint(State, *V);
+ ActualTaintedParams.push_back({Param.Value, V.getValue(), Param.Index});
+ }
}
// Clear up the taint info from the state.
State = State->remove<TaintArgsOnPostVisit>(CurrentFrame);
- C.addTransition(State);
+ const NoteTag *Note = [&]() -> const NoteTag * {
+ if (ActualTaintedParams.empty())
+ return nullptr;
+ return C.getNoteTag(
+ [ActualTaintedParams](PathSensitiveBugReport &BR) -> std::string {
+ const auto Interesting = [&BR](const auto &Param) -> bool {
+ return BR.isInteresting(Param.PostValue);
+ };
+
+ auto InterestingTaintedParams =
+ llvm::make_filter_range(ActualTaintedParams, Interesting);
+
+ if (InterestingTaintedParams.empty())
+ return "";
+
+ SmallVector<std::string, 6> MsgParts;
+ for (const auto &Param : InterestingTaintedParams) {
+ BR.markInteresting(Param.PreValue);
+ const auto ParamOrdinal = Param.Index + 1;
+ MsgParts.emplace_back((Twine(std::to_string(ParamOrdinal)) +
+ llvm::getOrdinalSuffix(ParamOrdinal))
+ .str());
+ }
+
+ return (Twine("Propagated taint to the ") +
+ llvm::join(std::move(MsgParts), ", ") +
+ (MsgParts.size() == 1 ? " parameter" : " parameters"))
+ .str();
+ });
+ }();
+
+ C.addTransition(State, Pred, Note);
}
void GenericTaintChecker::printState(raw_ostream &Out, ProgramStateRef State,
@@ -856,11 +928,18 @@
/// A rule is relevant if PropSrcArgs is empty, or if any of its signified
/// args are tainted in context of the current CallEvent.
bool IsMatching = PropSrcArgs.isEmpty();
- ForEachCallArg(
- [this, &C, &IsMatching, &State](ArgIdxTy I, const Expr *E, SVal) {
- IsMatching = IsMatching || (PropSrcArgs.contains(I) &&
- isTaintedOrPointsToTainted(E, State, C));
- });
+ ArgIdxTy TaintedArgIdx;
+ ForEachCallArg([this, &C, &IsMatching, &State,
+ &TaintedArgIdx](ArgIdxTy I, const Expr *E, SVal) {
+ if (IsMatching)
+ return;
+
+ IsMatching =
+ PropSrcArgs.contains(I) && isTaintedOrPointsToTainted(E, State, C);
+
+ if (IsMatching)
+ TaintedArgIdx = I;
+ });
if (!IsMatching)
return;
@@ -876,28 +955,39 @@
return IsNonConstRef || IsNonConstPtr;
};
+ struct Pack {
+ SVal PreValue;
+ SVal PostValue;
+ unsigned Index;
+ };
+ SmallVector<Pack, 6> ActualTaintedParams;
+
/// Propagate taint where it is necessary.
- auto &F = State->getStateManager().get_context<ArgIdxFactory>();
- ImmutableSet<ArgIdxTy> Result = F.getEmptySet();
+ auto &F = State->getStateManager().get_context<FunctionParamFactory>();
+ ImmutableList<FunctionParameter> Result = F.getEmptyList();
ForEachCallArg(
[&](ArgIdxTy I, const Expr *E, SVal V) {
if (PropDstArgs.contains(I)) {
LLVM_DEBUG(llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs());
llvm::dbgs()
<< "> prepares tainting arg index: " << I << '\n';);
- Result = F.add(Result, I);
+ if (I == ReturnValueIndex)
+ State = State->set<ReturnsTainted>(true);
+ else
+ Result = F.add<FunctionParameter>({V, I}, Result);
+ return;
}
// TODO: We should traverse all reachable memory regions via the
// escaping parameter. Instead of doing that we simply mark only the
// referred memory region as tainted.
if (WouldEscape(V, E->getType())) {
- LLVM_DEBUG(if (!Result.contains(I)) {
+ LLVM_DEBUG(if (!Result.contains({V, I})) {
llvm::dbgs() << "PreCall<";
Call.dump(llvm::dbgs());
llvm::dbgs() << "> prepares tainting arg index: " << I << '\n';
});
- Result = F.add(Result, I);
+ Result = F.add<FunctionParameter>({V, I}, Result);
}
});
@@ -924,6 +1014,7 @@
if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
auto report = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
report->addRange(E->getSourceRange());
+ report->markInteresting(*TaintedSVal);
report->addVisitor(std::make_unique<TaintBugVisitor>(*TaintedSVal));
C.emitReport(std::move(report));
return true;
@@ -994,9 +1085,9 @@
return;
ProgramStateRef State = C.getState();
- auto &F = State->getStateManager().get_context<ArgIdxFactory>();
- ImmutableSet<ArgIdxTy> Result = F.add(F.getEmptySet(), ReturnValueIndex);
- State = State->set<TaintArgsOnPostVisit>(C.getStackFrame(), Result);
+ State = State->set<ReturnsTainted>(true);
+ auto &F = State->getStateManager().get_context<FunctionParamFactory>();
+ State = State->set<TaintArgsOnPostVisit>(C.getStackFrame(), F.create<FunctionParameter>({Call.getReturnValue(), ReturnValueIndex}));
C.addTransition(State);
}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -14,6 +14,7 @@
#include "InterCheckerAPI.h"
#include "clang/Basic/CharInfo.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -55,7 +56,8 @@
enum class AccessKind { write, read };
static ErrorMessage createOutOfBoundErrorMsg(StringRef FunctionDescription,
- AccessKind Access) {
+ AccessKind Access,
+ bool IsTainted = false) {
ErrorMessage Message;
llvm::raw_svector_ostream Os(Message);
@@ -64,9 +66,9 @@
<< &FunctionDescription.data()[1];
if (Access == AccessKind::write) {
- Os << " overflows the destination buffer";
+ Os << (IsTainted ? " might" : "") << " overflows the destination buffer";
} else { // read access
- Os << " accesses out-of-bound array element";
+ Os << (IsTainted ? " might" : "") << " accesses out-of-bound array element";
}
return Message;
@@ -94,6 +96,9 @@
bool CheckCStringNotNullTerm = false;
bool CheckCStringUninitializedRead = false;
+ // Taint makes no sense for NullArg, BufferOverlap and NotNullTerm checks.
+ bool ConsiderTaintForCStringOutOfBounds = false;
+
CheckerNameRef CheckNameCStringNullArg;
CheckerNameRef CheckNameCStringOutOfBounds;
CheckerNameRef CheckNameCStringBufferOverlap;
@@ -369,6 +374,15 @@
createOutOfBoundErrorMsg(CurrentFunctionDescription, Access);
emitOutOfBoundsBug(C, StOutBound, Buffer.Expression, Message);
return nullptr;
+ } else if (Filter.CheckCStringOutOfBounds &&
+ Filter.ConsiderTaintForCStringOutOfBounds && StOutBound &&
+ StInBound &&
+ (taint::isTainted(state, Idx) || taint::isTainted(state, Size))) {
+ // FIXME: Explain whether the index or the extent was tainted.
+ // FIXME: Add a visitor explaining where the taint was introduced.
+ ErrorMessage Message = createOutOfBoundErrorMsg(CurrentFunctionDescription,
+ Access, /*IsTainted=*/true);
+ emitOutOfBoundsBug(C, StOutBound, Buffer.Expression, Message);
}
// Ensure that we wouldn't read uninitialized value.
@@ -2489,8 +2503,20 @@
\
bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; }
+#define REGISTER_CHECKER_WITH_TAINT(name) \
+ void ento::register##name(CheckerManager &mgr) { \
+ CStringChecker *checker = mgr.getChecker<CStringChecker>(); \
+ checker->Filter.Check##name = true; \
+ checker->Filter.CheckName##name = mgr.getCurrentCheckerName(); \
+ checker->Filter.ConsiderTaintFor##name = \
+ mgr.getAnalyzerOptions().getCheckerBooleanOption( \
+ mgr.getCurrentCheckerName().getName(), "ConsiderTaint"); \
+ } \
+ \
+ bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; }
+
REGISTER_CHECKER(CStringNullArg)
-REGISTER_CHECKER(CStringOutOfBounds)
REGISTER_CHECKER(CStringBufferOverlap)
REGISTER_CHECKER(CStringNotNullTerm)
REGISTER_CHECKER(CStringUninitializedRead)
+REGISTER_CHECKER_WITH_TAINT(CStringOutOfBounds)
Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -36,6 +36,7 @@
enum OOB_Kind { OOB_Precedes, OOB_Excedes, OOB_Tainted };
void reportOOB(CheckerContext &C, ProgramStateRef errorState, OOB_Kind kind,
+ SVal InterestingValue = UndefinedVal(),
std::unique_ptr<BugReporterVisitor> Visitor = nullptr) const;
public:
@@ -205,10 +206,10 @@
// If we are under constrained and the index variables are tainted, report.
if (state_exceedsUpperBound && state_withinUpperBound) {
- SVal ByteOffset = rawOffset.getByteOffset();
- if (isTainted(state, ByteOffset)) {
+ if (isTainted(state, *upperboundToCheck)) {
reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted,
- std::make_unique<TaintBugVisitor>(ByteOffset));
+ *upperboundToCheck,
+ std::make_unique<TaintBugVisitor>(*upperboundToCheck));
return;
}
} else if (state_exceedsUpperBound) {
@@ -229,7 +230,7 @@
void ArrayBoundCheckerV2::reportOOB(
CheckerContext &checkerContext, ProgramStateRef errorState, OOB_Kind kind,
- std::unique_ptr<BugReporterVisitor> Visitor) const {
+ SVal InterestingValue, std::unique_ptr<BugReporterVisitor> Visitor) const {
ExplodedNode *errorNode = checkerContext.generateErrorNode(errorState);
if (!errorNode)
@@ -257,6 +258,7 @@
}
auto BR = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), errorNode);
+ BR->markInteresting(InterestingValue);
BR->addVisitor(std::move(Visitor));
checkerContext.emitReport(std::move(BR));
}
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -471,6 +471,16 @@
def CStringOutOfBounds : Checker<"OutOfBounds">,
HelpText<"Check for out-of-bounds access in string functions">,
Dependencies<[CStringModeling]>,
+ CheckerOptions<[
+ CmdLineOption<Boolean,
+ "ConsiderTaint",
+ "If set to true and the access under test depends on tainted "
+ "value we will emit a warning even if we can not ensure that "
+ "we encountered a bug",
+ "false",
+ Released,
+ Hide>
+ ]>,
Documentation<HasDocumentation>;
def CStringBufferOverlap : Checker<"BufferOverlap">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
