https://github.com/dkrupp updated
https://github.com/llvm/llvm-project/pull/92420
>From 80767176cbe8e5717c5f42b113f305d81b635cb9 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.kr...@ericsson.com>
Date: Tue, 30 Apr 2024 15:20:52 +0200
Subject: [PATCH 1/2] [analyzer] Adding taint analysis capability to
unix.Malloc checker
unix.Malloc checker will warn if a memory allocation function
(malloc, calloc, realloc, alloca) is called with a tainted
(attacker controlled) size parameter.
A large, maliciously set size value can trigger memory exhaustion.
To get this warning, the alpha.security.taint.TaintPropagation checker
also needs to be switched on.
The warning will only be emitted, if the analyzer cannot prove
that the size is below reasonable bounds (<SIZE_MAX/4).
---
clang/docs/analyzer/checkers.rst | 35 +++++++
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 96 ++++++++++++++++---
clang/test/Analysis/malloc.c | 42 +++++++-
3 files changed, 159 insertions(+), 14 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index eb8b58323da4d..0cdf6dcab2875 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1273,6 +1273,41 @@ Check for memory leaks, double free, and use-after-free
problems. Traces memory
.. literalinclude:: checkers/unix_malloc_example.c
:language: c
+If the ``alpha.security.taint.TaintPropagation`` checker is enabled, the
checker
+warns for cases when the ``size`` parameter of the ``malloc`` , ``calloc``,
+``realloc``, ``alloca`` is tainted (potentially attacker controlled). If an
+attacker can inject a large value as the size parameter, memory exhaustion
+denial of service attack can be carried out.
+
+The analyzer emits warning only if it cannot prove that the size parameter is
+within reasonable bounds (``<= SIZE_MAX/4``). This functionality partially
+covers the SEI Cert coding standard rule `INT04-C
+<https://wiki.sei.cmu.edu/confluence/display/c/INT04-C.+Enforce+limits+on+integer+values+originating+from+tainted+sources>`_.
+
+You can silence this warning either by bound checking the ``size`` parameter,
or
+by explicitly marking the ``size`` parameter as sanitized. See the
+:ref:`alpha-security-taint-TaintPropagation` checker for more details.
+
+.. code-block:: c
+
+ void t1(void) {
+ size_t size;
+ scanf("%zu", &size);
+ int *p = malloc(size); // warn: malloc is called with a tainted
(potentially attacker controlled) value
+ free(p);
+ }
+
+ void t3(void) {
+ size_t size;
+ scanf("%zu", &size);
+ if (1024<size)
+ return;
+ int *p = malloc(size); // No warning expected as the the user input is
bound
+ free(p);
+ }
+
+.. _unix-MismatchedDeallocator:
+
.. _unix-MallocSizeof:
unix.MallocSizeof (C)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index dd204b62dcc04..2cc9205c07814 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/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"
@@ -365,6 +366,7 @@ class MallocChecker
mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds];
+ mutable std::unique_ptr<BugType> BT_TaintedAlloc[CK_NumCheckKinds];
#define CHECK_FN(NAME)
\
void NAME(const CallEvent &Call, CheckerContext &C) const;
@@ -462,6 +464,13 @@ class MallocChecker
};
bool isMemCall(const CallEvent &Call) const;
+ void reportTaintBug(StringRef Msg, ProgramStateRef State, CheckerContext &C,
+ llvm::ArrayRef<SymbolRef> TaintedSyms,
+ AllocationFamily Family, const Expr *SizeEx) const;
+
+ void CheckTaintedness(CheckerContext &C, const CallEvent &Call,
+ const SVal SizeSVal, ProgramStateRef State,
+ AllocationFamily Family) const;
// TODO: Remove mutable by moving the initializtaion to the registry
function.
mutable std::optional<uint64_t> KernelZeroFlagVal;
@@ -521,9 +530,9 @@ class MallocChecker
/// malloc leaves it undefined.
/// \param [in] State The \c ProgramState right before allocation.
/// \returns The ProgramState right after allocation.
- [[nodiscard]] static ProgramStateRef
+ [[nodiscard]] ProgramStateRef
MallocMemAux(CheckerContext &C, const CallEvent &Call, const Expr *SizeEx,
- SVal Init, ProgramStateRef State, AllocationFamily Family);
+ SVal Init, ProgramStateRef State, AllocationFamily Family)
const;
/// Models memory allocation.
///
@@ -534,9 +543,10 @@ class MallocChecker
/// malloc leaves it undefined.
/// \param [in] State The \c ProgramState right before allocation.
/// \returns The ProgramState right after allocation.
- [[nodiscard]] static ProgramStateRef
- MallocMemAux(CheckerContext &C, const CallEvent &Call, SVal Size, SVal Init,
- ProgramStateRef State, AllocationFamily Family);
+ [[nodiscard]] ProgramStateRef MallocMemAux(CheckerContext &C,
+ const CallEvent &Call, SVal Size,
+ SVal Init, ProgramStateRef State,
+ AllocationFamily Family) const;
// Check if this malloc() for special flags. At present that means M_ZERO or
// __GFP_ZERO (in which case, treat it like calloc).
@@ -649,8 +659,9 @@ class MallocChecker
/// \param [in] Call The expression that reallocated memory
/// \param [in] State The \c ProgramState right before reallocation.
/// \returns The ProgramState right after allocation.
- [[nodiscard]] static ProgramStateRef
- CallocMem(CheckerContext &C, const CallEvent &Call, ProgramStateRef State);
+ [[nodiscard]] ProgramStateRef CallocMem(CheckerContext &C,
+ const CallEvent &Call,
+ ProgramStateRef State) const;
/// See if deallocation happens in a suspicious context. If so, escape the
/// pointers that otherwise would have been deallocated and return true.
@@ -1779,7 +1790,7 @@ ProgramStateRef
MallocChecker::MallocMemAux(CheckerContext &C,
const CallEvent &Call,
const Expr *SizeEx, SVal Init,
ProgramStateRef State,
- AllocationFamily Family) {
+ AllocationFamily Family) const {
if (!State)
return nullptr;
@@ -1787,10 +1798,71 @@ ProgramStateRef
MallocChecker::MallocMemAux(CheckerContext &C,
return MallocMemAux(C, Call, C.getSVal(SizeEx), Init, State, Family);
}
+void MallocChecker::reportTaintBug(StringRef Msg, ProgramStateRef State,
+ CheckerContext &C,
+ llvm::ArrayRef<SymbolRef> TaintedSyms,
+ AllocationFamily Family,
+ const Expr *SizeEx) const {
+ if (ExplodedNode *N = C.generateErrorNode(State)) {
+
+ std::optional<MallocChecker::CheckKind> CheckKind =
+ getCheckIfTracked(Family);
+ if (!CheckKind)
+ return;
+ if (!BT_TaintedAlloc[*CheckKind])
+ BT_TaintedAlloc[*CheckKind].reset(new BugType(CheckNames[*CheckKind],
+ "Tainted Memory
Allocation",
+ categories::MemoryError));
+ auto R = std::make_unique<PathSensitiveBugReport>(
+ *BT_TaintedAlloc[*CheckKind], Msg, N);
+
+ bugreporter::trackExpressionValue(N, SizeEx, *R);
+ for (auto Sym : TaintedSyms)
+ R->markInteresting(Sym);
+ C.emitReport(std::move(R));
+ }
+}
+
+void MallocChecker::CheckTaintedness(CheckerContext &C, const CallEvent &Call,
+ const SVal SizeSVal, ProgramStateRef
State,
+ AllocationFamily Family) const {
+ std::vector<SymbolRef> TaintedSyms =
+ clang::ento::taint::getTaintedSymbols(State, SizeSVal);
+ if (!TaintedSyms.empty()) {
+ SValBuilder &SVB = C.getSValBuilder();
+ QualType SizeTy = SVB.getContext().getSizeType();
+ QualType CmpTy = SVB.getConditionType();
+ // In case the symbol is tainted, we give a warning if the
+ // size is larger than SIZE_MAX/4
+ BasicValueFactory &BVF = SVB.getBasicValueFactory();
+ const llvm::APSInt MaxValInt = BVF.getMaxValue(SizeTy);
+ NonLoc MaxLength =
+ SVB.makeIntVal(MaxValInt / APSIntType(MaxValInt).getValue(4));
+ std::optional<NonLoc> SizeNL = SizeSVal.getAs<NonLoc>();
+ auto Cmp = SVB.evalBinOpNN(State, BO_GE, *SizeNL, MaxLength, CmpTy)
+ .getAs<DefinedOrUnknownSVal>();
+ if (!Cmp)
+ return;
+ auto [StateTooLarge, StateNotTooLarge] = State->assume(*Cmp);
+ if (!StateTooLarge && StateNotTooLarge) {
+ // we can prove that size is not too large so ok.
+ return;
+ }
+
+ std::string Callee = "Memory allocation function";
+ if (Call.getCalleeIdentifier())
+ Callee = Call.getCalleeIdentifier()->getName().str();
+ reportTaintBug(
+ Callee + " is called with a tainted (potentially attacker controlled) "
+ "value. Make sure the value is bound checked.",
+ State, C, TaintedSyms, Family, Call.getArgExpr(0));
+ }
+}
+
ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
const CallEvent &Call, SVal Size,
SVal Init, ProgramStateRef State,
- AllocationFamily Family) {
+ AllocationFamily Family) const {
if (!State)
return nullptr;
@@ -1819,9 +1891,7 @@ ProgramStateRef
MallocChecker::MallocMemAux(CheckerContext &C,
if (Size.isUndef())
Size = UnknownVal();
- // TODO: If Size is tainted and we cannot prove that it is within
- // reasonable bounds, emit a warning that an attacker may
- // provoke a memory exhaustion error.
+ CheckTaintedness(C, Call, Size, State, AF_Malloc);
// Set the region's extent.
State = setDynamicExtent(State, RetVal.getAsRegion(),
@@ -2761,7 +2831,7 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const
CallEvent &Call,
ProgramStateRef MallocChecker::CallocMem(CheckerContext &C,
const CallEvent &Call,
- ProgramStateRef State) {
+ ProgramStateRef State) const {
if (!State)
return nullptr;
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index e5cb45ba73352..6dba76a57d83f 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -3,7 +3,8 @@
// RUN: -analyzer-checker=alpha.deadcode.UnreachableCode \
// RUN: -analyzer-checker=alpha.core.CastSize \
// RUN: -analyzer-checker=unix \
-// RUN: -analyzer-checker=debug.ExprInspection
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-checker=alpha.security.taint.TaintPropagation
#include "Inputs/system-header-simulator.h"
@@ -48,6 +49,45 @@ void myfoo(int *p);
void myfooint(int p);
char *fooRetPtr(void);
+void t1(void) {
+ size_t size;
+ scanf("%zu", &size);
+ int *p = malloc(size); // expected-warning{{malloc is called with a tainted
(potentially attacker controlled) value}}
+ free(p);
+}
+
+void t2(void) {
+ size_t size;
+ scanf("%zu", &size);
+ int *p = calloc(size,2); // expected-warning{{calloc is called with a
tainted (potentially attacker controlled) value}}
+ free(p);
+}
+
+void t3(void) {
+ size_t size;
+ scanf("%zu", &size);
+ if (1024<size)
+ return;
+ int *p = malloc(size); // No warning expected as the the user input is bound
+ free(p);
+}
+
+void t4(void) {
+ size_t size;
+ int *p = malloc(sizeof(int));
+ scanf("%zu", &size);
+ p = (int*) realloc((void*) p, size); // // expected-warning{{realloc is
called with a tainted (potentially attacker controlled) value}}
+ free(p);
+}
+
+void t5(void) {
+ size_t size;
+ int *p = alloca(sizeof(int));
+ scanf("%zu", &size);
+ p = (int*) alloca(size); // // expected-warning{{alloca is called with a
tainted (potentially attacker controlled) value}}
+}
+
+
void f1(void) {
int *p = malloc(12);
return; // expected-warning{{Potential leak of memory pointed to by 'p'}}
>From b31ec694c88635404b252f00472140e83083fd02 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.kr...@ericsson.com>
Date: Thu, 23 May 2024 10:12:48 +0200
Subject: [PATCH 2/2] Addressing review commnents
- Create a new optional checker optin.taint.TaintMalloc
- Add test case for testing taint diagnostic notes
---
clang/docs/analyzer/checkers.rst | 77 +++--
.../clang/StaticAnalyzer/Checkers/Checkers.td | 13 +
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 326 +++++++++---------
clang/test/Analysis/malloc.c | 27 +-
.../test/Analysis/taint-diagnostic-visitor.c | 12 +-
5 files changed, 243 insertions(+), 212 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 0cdf6dcab2875..827ba38f020fe 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -599,7 +599,47 @@ Warns when a nullable pointer is returned from a function
that has _Nonnull retu
optin
^^^^^
-Checkers for portability, performance or coding style specific rules.
+Checkers for portability, performance, optional security and coding style
specific rules.
+
+.. _optin-taint-TaintMalloc:
+
+optin.taint.TaintMalloc
+"""""""""""""""""""""""
+
+This checker warns for cases when the ``size`` parameter of the ``malloc`` ,
+``calloc``, ``realloc``, ``alloca`` is tainted (potentially attacker
controlled).
+If an attacker can inject a large value as the size parameter, memory
exhaustion
+denial of service attack can be carried out.
+
+The ``alpha.security.taint.TaintPropagation`` checker also needs to be enabled
for
+this checker to give warnings.
+
+The analyzer emits warning only if it cannot prove that the size parameter is
+within reasonable bounds (``<= SIZE_MAX/4``). This functionality partially
+covers the SEI Cert coding standard rule `INT04-C
+<https://wiki.sei.cmu.edu/confluence/display/c/INT04-C.+Enforce+limits+on+integer+values+originating+from+tainted+sources>`_.
+
+You can silence this warning either by bound checking the ``size`` parameter,
or
+by explicitly marking the ``size`` parameter as sanitized. See the
+:ref:`alpha-security-taint-TaintPropagation` checker for more details.
+
+.. code-block:: c
+
+ void t1(void) {
+ size_t size;
+ scanf("%zu", &size);
+ int *p = malloc(size); // warn: malloc is called with a tainted
(potentially attacker controlled) value
+ free(p);
+ }
+
+ void t3(void) {
+ size_t size = 0;
+ scanf("%zu", &size);
+ if (1024 < size)
+ return;
+ int *p = malloc(size); // No warning expected as the the user input is
bound
+ free(p);
+ }
.. _optin-core-EnumCastOutOfRange:
@@ -1273,41 +1313,6 @@ Check for memory leaks, double free, and use-after-free
problems. Traces memory
.. literalinclude:: checkers/unix_malloc_example.c
:language: c
-If the ``alpha.security.taint.TaintPropagation`` checker is enabled, the
checker
-warns for cases when the ``size`` parameter of the ``malloc`` , ``calloc``,
-``realloc``, ``alloca`` is tainted (potentially attacker controlled). If an
-attacker can inject a large value as the size parameter, memory exhaustion
-denial of service attack can be carried out.
-
-The analyzer emits warning only if it cannot prove that the size parameter is
-within reasonable bounds (``<= SIZE_MAX/4``). This functionality partially
-covers the SEI Cert coding standard rule `INT04-C
-<https://wiki.sei.cmu.edu/confluence/display/c/INT04-C.+Enforce+limits+on+integer+values+originating+from+tainted+sources>`_.
-
-You can silence this warning either by bound checking the ``size`` parameter,
or
-by explicitly marking the ``size`` parameter as sanitized. See the
-:ref:`alpha-security-taint-TaintPropagation` checker for more details.
-
-.. code-block:: c
-
- void t1(void) {
- size_t size;
- scanf("%zu", &size);
- int *p = malloc(size); // warn: malloc is called with a tainted
(potentially attacker controlled) value
- free(p);
- }
-
- void t3(void) {
- size_t size;
- scanf("%zu", &size);
- if (1024<size)
- return;
- int *p = malloc(size); // No warning expected as the the user input is
bound
- free(p);
- }
-
-.. _unix-MismatchedDeallocator:
-
.. _unix-MallocSizeof:
unix.MallocSizeof (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 520286b57c9fd..a79032c655c10 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -36,6 +36,8 @@ def CoreAlpha : Package<"core">, ParentPackage<Alpha>;
// Note: OptIn is *not* intended for checkers that are too noisy to be on by
// default. Such checkers belong in the alpha package.
def OptIn : Package<"optin">;
+def TaintOptIn : Package<"taint">, ParentPackage<OptIn>;
+
def CoreOptIn : Package<"core">, ParentPackage<OptIn>;
// In the Portability package reside checkers for finding code that relies on
@@ -452,6 +454,7 @@ def EnumCastOutOfRangeChecker :
Checker<"EnumCastOutOfRange">,
} // end "optin.core"
+
//===----------------------------------------------------------------------===//
// Unix API checkers.
//===----------------------------------------------------------------------===//
@@ -625,6 +628,16 @@ def BlockInCriticalSectionChecker :
Checker<"BlockInCriticalSection">,
} // end "alpha.unix"
+let ParentPackage = TaintOptIn in {
+
+def TaintMallocChecker: Checker<"TaintMalloc">,
+ HelpText<"Check for memory allocations, where the size parameter "
+ "might be a tainted (attacker controlled) value.">,
+ Dependencies<[DynamicMemoryModeling]>,
+ Documentation<HasDocumentation>;
+
+} // end "optin.taint"
+
//===----------------------------------------------------------------------===//
// C++ checkers.
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 2cc9205c07814..0167dd133657f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -323,6 +323,7 @@ class MallocChecker
CK_NewDeleteLeaksChecker,
CK_MismatchedDeallocatorChecker,
CK_InnerPointerChecker,
+ CK_TaintMallocChecker,
CK_NumCheckKinds
};
@@ -366,7 +367,7 @@ class MallocChecker
mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds];
- mutable std::unique_ptr<BugType> BT_TaintedAlloc[CK_NumCheckKinds];
+ mutable std::unique_ptr<BugType> BT_TaintedAlloc;
#define CHECK_FN(NAME)
\
void NAME(const CallEvent &Call, CheckerContext &C) const;
@@ -1803,32 +1804,34 @@ void MallocChecker::reportTaintBug(StringRef Msg,
ProgramStateRef State,
llvm::ArrayRef<SymbolRef> TaintedSyms,
AllocationFamily Family,
const Expr *SizeEx) const {
- if (ExplodedNode *N = C.generateErrorNode(State)) {
- std::optional<MallocChecker::CheckKind> CheckKind =
- getCheckIfTracked(Family);
- if (!CheckKind)
- return;
- if (!BT_TaintedAlloc[*CheckKind])
- BT_TaintedAlloc[*CheckKind].reset(new BugType(CheckNames[*CheckKind],
- "Tainted Memory
Allocation",
- categories::MemoryError));
- auto R = std::make_unique<PathSensitiveBugReport>(
- *BT_TaintedAlloc[*CheckKind], Msg, N);
+ if (!ChecksEnabled[CK_TaintMallocChecker])
+ return;
+ if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) {
+ if (!BT_TaintedAlloc)
+ BT_TaintedAlloc.reset(new BugType(CheckNames[CK_TaintMallocChecker],
+ "Tainted Memory Allocation",
+ categories::TaintedData));
+ auto R = std::make_unique<PathSensitiveBugReport>(*BT_TaintedAlloc, Msg,
N);
+
+ R->addRange(SizeEx->getSourceRange());
bugreporter::trackExpressionValue(N, SizeEx, *R);
- for (auto Sym : TaintedSyms)
- R->markInteresting(Sym);
+ for (auto TaintedSym : TaintedSyms) {
+ R->markInteresting(TaintedSym);
+ }
C.emitReport(std::move(R));
}
}
-void MallocChecker::CheckTaintedness(CheckerContext &C, const CallEvent &Call,
- const SVal SizeSVal, ProgramStateRef
State,
- AllocationFamily Family) const {
- std::vector<SymbolRef> TaintedSyms =
- clang::ento::taint::getTaintedSymbols(State, SizeSVal);
- if (!TaintedSyms.empty()) {
+void MallocChecker::CheckTaintedness(
+ CheckerContext & C, const CallEvent &Call, const SVal SizeSVal,
+ ProgramStateRef State, AllocationFamily Family) const {
+ std::vector<SymbolRef> TaintedSyms =
+ taint::getTaintedSymbols(State, SizeSVal);
+ if (TaintedSyms.empty())
+ return;
+
SValBuilder &SVB = C.getSValBuilder();
QualType SizeTy = SVB.getContext().getSizeType();
QualType CmpTy = SVB.getConditionType();
@@ -1857,178 +1860,176 @@ void MallocChecker::CheckTaintedness(CheckerContext
&C, const CallEvent &Call,
"value. Make sure the value is bound checked.",
State, C, TaintedSyms, Family, Call.getArgExpr(0));
}
-}
-ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
- const CallEvent &Call, SVal Size,
- SVal Init, ProgramStateRef State,
- AllocationFamily Family) const {
- if (!State)
- return nullptr;
+ ProgramStateRef MallocChecker::MallocMemAux(
+ CheckerContext & C, const CallEvent &Call, SVal Size, SVal Init,
+ ProgramStateRef State, AllocationFamily Family) const {
+ if (!State)
+ return nullptr;
- const Expr *CE = Call.getOriginExpr();
+ const Expr *CE = Call.getOriginExpr();
- // We expect the malloc functions to return a pointer.
- if (!Loc::isLocType(CE->getType()))
- return nullptr;
+ // We expect the malloc functions to return a pointer.
+ if (!Loc::isLocType(CE->getType()))
+ return nullptr;
- // Bind the return value to the symbolic value from the heap region.
- // TODO: move use of this functions to an EvalCall callback, becasue
- // BindExpr() should'nt be used elsewhere.
- unsigned Count = C.blockCount();
- SValBuilder &SVB = C.getSValBuilder();
- const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
- DefinedSVal RetVal =
- ((Family == AF_Alloca) ? SVB.getAllocaRegionVal(CE, LCtx, Count)
- : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count)
- .castAs<DefinedSVal>());
- State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ // Bind the return value to the symbolic value from the heap region.
+ // TODO: move use of this functions to an EvalCall callback, becasue
+ // BindExpr() should'nt be used elsewhere.
+ unsigned Count = C.blockCount();
+ SValBuilder &SVB = C.getSValBuilder();
+ const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+ DefinedSVal RetVal =
+ ((Family == AF_Alloca) ? SVB.getAllocaRegionVal(CE, LCtx, Count)
+ : SVB.getConjuredHeapSymbolVal(CE, LCtx, Count)
+ .castAs<DefinedSVal>());
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
- // Fill the region with the initialization value.
- State = State->bindDefaultInitial(RetVal, Init, LCtx);
+ // Fill the region with the initialization value.
+ State = State->bindDefaultInitial(RetVal, Init, LCtx);
- // If Size is somehow undefined at this point, this line prevents a crash.
- if (Size.isUndef())
- Size = UnknownVal();
+ // If Size is somehow undefined at this point, this line prevents a crash.
+ if (Size.isUndef())
+ Size = UnknownVal();
- CheckTaintedness(C, Call, Size, State, AF_Malloc);
+ CheckTaintedness(C, Call, Size, State, AF_Malloc);
- // Set the region's extent.
- State = setDynamicExtent(State, RetVal.getAsRegion(),
- Size.castAs<DefinedOrUnknownSVal>(), SVB);
+ // Set the region's extent.
+ State = setDynamicExtent(State, RetVal.getAsRegion(),
+ Size.castAs<DefinedOrUnknownSVal>(), SVB);
- return MallocUpdateRefState(C, CE, State, Family);
-}
+ return MallocUpdateRefState(C, CE, State, Family);
+ }
-static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E,
- ProgramStateRef State,
- AllocationFamily Family,
- std::optional<SVal> RetVal) {
- if (!State)
- return nullptr;
+ static ProgramStateRef MallocUpdateRefState(
+ CheckerContext & C, const Expr *E, ProgramStateRef State,
+ AllocationFamily Family, std::optional<SVal> RetVal) {
+ if (!State)
+ return nullptr;
- // Get the return value.
- if (!RetVal)
- RetVal = C.getSVal(E);
+ // Get the return value.
+ if (!RetVal)
+ RetVal = C.getSVal(E);
- // We expect the malloc functions to return a pointer.
- if (!RetVal->getAs<Loc>())
- return nullptr;
+ // We expect the malloc functions to return a pointer.
+ if (!RetVal->getAs<Loc>())
+ return nullptr;
- SymbolRef Sym = RetVal->getAsLocSymbol();
+ SymbolRef Sym = RetVal->getAsLocSymbol();
- // This is a return value of a function that was not inlined, such as
malloc()
- // or new(). We've checked that in the caller. Therefore, it must be a
symbol.
- assert(Sym);
- // FIXME: In theory this assertion should fail for `alloca()` calls (because
- // `AllocaRegion`s are not symbolic); but in practice this does not happen.
- // As the current code appears to work correctly, I'm not touching this issue
- // now, but it would be good to investigate and clarify this.
- // Also note that perhaps the special `AllocaRegion` should be replaced by
- // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to enable
- // proper tracking of memory allocated by `alloca()` -- and after that change
- // this assertion would become valid again.
-
- // Set the symbol's state to Allocated.
- return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
-}
-
-ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
- const CallEvent &Call,
- const OwnershipAttr *Att,
- ProgramStateRef State) const {
- if (!State)
- return nullptr;
+ // This is a return value of a function that was not inlined, such as
+ // malloc() or new(). We've checked that in the caller. Therefore, it must
+ // be a symbol.
+ assert(Sym);
+ // FIXME: In theory this assertion should fail for `alloca()` calls
(because
+ // `AllocaRegion`s are not symbolic); but in practice this does not happen.
+ // As the current code appears to work correctly, I'm not touching this
+ // issue now, but it would be good to investigate and clarify this. Also
+ // note that perhaps the special `AllocaRegion` should be replaced by
+ // `SymbolicRegion` (or turned into a subclass of `SymbolicRegion`) to
+ // enable proper tracking of memory allocated by `alloca()` -- and after
+ // that change this assertion would become valid again.
+
+ // Set the symbol's state to Allocated.
+ return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
+ }
+
+ ProgramStateRef MallocChecker::FreeMemAttr(
+ CheckerContext & C, const CallEvent &Call, const OwnershipAttr *Att,
+ ProgramStateRef State) const {
+ if (!State)
+ return nullptr;
- if (Att->getModule()->getName() != "malloc")
- return nullptr;
+ if (Att->getModule()->getName() != "malloc")
+ return nullptr;
- bool IsKnownToBeAllocated = false;
+ bool IsKnownToBeAllocated = false;
- for (const auto &Arg : Att->args()) {
- ProgramStateRef StateI =
- FreeMemAux(C, Call, State, Arg.getASTIndex(),
- Att->getOwnKind() == OwnershipAttr::Holds,
- IsKnownToBeAllocated, AF_Malloc);
- if (StateI)
- State = StateI;
+ for (const auto &Arg : Att->args()) {
+ ProgramStateRef StateI =
+ FreeMemAux(C, Call, State, Arg.getASTIndex(),
+ Att->getOwnKind() == OwnershipAttr::Holds,
+ IsKnownToBeAllocated, AF_Malloc);
+ if (StateI)
+ State = StateI;
+ }
+ return State;
}
- return State;
-}
-
-ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
- const CallEvent &Call,
- ProgramStateRef State, unsigned Num,
- bool Hold, bool
&IsKnownToBeAllocated,
- AllocationFamily Family,
- bool ReturnsNullOnFailure) const {
- if (!State)
- return nullptr;
- if (Call.getNumArgs() < (Num + 1))
- return nullptr;
+ ProgramStateRef MallocChecker::FreeMemAux(
+ CheckerContext & C, const CallEvent &Call, ProgramStateRef State,
+ unsigned Num, bool Hold, bool &IsKnownToBeAllocated,
+ AllocationFamily Family, bool ReturnsNullOnFailure) const {
+ if (!State)
+ return nullptr;
- return FreeMemAux(C, Call.getArgExpr(Num), Call, State, Hold,
- IsKnownToBeAllocated, Family, ReturnsNullOnFailure);
-}
+ if (Call.getNumArgs() < (Num + 1))
+ return nullptr;
-/// Checks if the previous call to free on the given symbol failed - if free
-/// failed, returns true. Also, returns the corresponding return value symbol.
-static bool didPreviousFreeFail(ProgramStateRef State,
- SymbolRef Sym, SymbolRef &RetStatusSymbol) {
- const SymbolRef *Ret = State->get<FreeReturnValue>(Sym);
- if (Ret) {
- assert(*Ret && "We should not store the null return symbol");
- ConstraintManager &CMgr = State->getConstraintManager();
- ConditionTruthVal FreeFailed = CMgr.isNull(State, *Ret);
- RetStatusSymbol = *Ret;
- return FreeFailed.isConstrainedTrue();
+ return FreeMemAux(C, Call.getArgExpr(Num), Call, State, Hold,
+ IsKnownToBeAllocated, Family, ReturnsNullOnFailure);
+ }
+
+ /// Checks if the previous call to free on the given symbol failed - if free
+ /// failed, returns true. Also, returns the corresponding return value
symbol.
+ static bool didPreviousFreeFail(ProgramStateRef State, SymbolRef Sym,
+ SymbolRef & RetStatusSymbol) {
+ const SymbolRef *Ret = State->get<FreeReturnValue>(Sym);
+ if (Ret) {
+ assert(*Ret && "We should not store the null return symbol");
+ ConstraintManager &CMgr = State->getConstraintManager();
+ ConditionTruthVal FreeFailed = CMgr.isNull(State, *Ret);
+ RetStatusSymbol = *Ret;
+ return FreeFailed.isConstrainedTrue();
+ }
+ return false;
}
- return false;
-}
-static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E) {
- if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
- // FIXME: This doesn't handle indirect calls.
- const FunctionDecl *FD = CE->getDirectCallee();
- if (!FD)
- return false;
+ static bool printMemFnName(raw_ostream & os, CheckerContext & C,
+ const Expr *E) {
+ if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
+ // FIXME: This doesn't handle indirect calls.
+ const FunctionDecl *FD = CE->getDirectCallee();
+ if (!FD)
+ return false;
- os << *FD;
- if (!FD->isOverloadedOperator())
- os << "()";
- return true;
- }
+ os << *FD;
+ if (!FD->isOverloadedOperator())
+ os << "()";
+ return true;
+ }
- if (const ObjCMessageExpr *Msg = dyn_cast<ObjCMessageExpr>(E)) {
- if (Msg->isInstanceMessage())
- os << "-";
- else
- os << "+";
- Msg->getSelector().print(os);
- return true;
- }
+ if (const ObjCMessageExpr *Msg = dyn_cast<ObjCMessageExpr>(E)) {
+ if (Msg->isInstanceMessage())
+ os << "-";
+ else
+ os << "+";
+ Msg->getSelector().print(os);
+ return true;
+ }
- if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
- os << "'"
- << getOperatorSpelling(NE->getOperatorNew()->getOverloadedOperator())
- << "'";
- return true;
- }
+ if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
+ os << "'"
+ << getOperatorSpelling(NE->getOperatorNew()->getOverloadedOperator())
+ << "'";
+ return true;
+ }
- if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E)) {
- os << "'"
- << getOperatorSpelling(DE->getOperatorDelete()->getOverloadedOperator())
- << "'";
- return true;
- }
+ if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E)) {
+ os << "'"
+ << getOperatorSpelling(
+ DE->getOperatorDelete()->getOverloadedOperator())
+ << "'";
+ return true;
+ }
- return false;
-}
+ return false;
+ }
-static void printExpectedAllocName(raw_ostream &os, AllocationFamily Family) {
+ static void printExpectedAllocName(raw_ostream & os,
+ AllocationFamily Family) {
- switch(Family) {
+ switch (Family) {
case AF_Malloc: os << "malloc()"; return;
case AF_CXXNew: os << "'new'"; return;
case AF_CXXNewArray: os << "'new[]'"; return;
@@ -3793,3 +3794,4 @@ REGISTER_CHECKER(MallocChecker)
REGISTER_CHECKER(NewDeleteChecker)
REGISTER_CHECKER(NewDeleteLeaksChecker)
REGISTER_CHECKER(MismatchedDeallocatorChecker)
+REGISTER_CHECKER(TaintMallocChecker)
diff --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 6dba76a57d83f..70b7f89a8d6d3 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -4,7 +4,8 @@
// RUN: -analyzer-checker=alpha.core.CastSize \
// RUN: -analyzer-checker=unix \
// RUN: -analyzer-checker=debug.ExprInspection \
-// RUN: -analyzer-checker=alpha.security.taint.TaintPropagation
+// RUN: -analyzer-checker=alpha.security.taint.TaintPropagation \
+// RUN: -analyzer-checker=optin.taint.TaintMalloc
#include "Inputs/system-header-simulator.h"
@@ -50,41 +51,41 @@ void myfooint(int p);
char *fooRetPtr(void);
void t1(void) {
- size_t size;
+ size_t size = 0;
scanf("%zu", &size);
int *p = malloc(size); // expected-warning{{malloc is called with a tainted
(potentially attacker controlled) value}}
free(p);
}
void t2(void) {
- size_t size;
+ size_t size = 0;
scanf("%zu", &size);
int *p = calloc(size,2); // expected-warning{{calloc is called with a
tainted (potentially attacker controlled) value}}
free(p);
}
void t3(void) {
- size_t size;
+ size_t size = 0;
scanf("%zu", &size);
- if (1024<size)
+ if (1024 < size)
return;
int *p = malloc(size); // No warning expected as the the user input is bound
free(p);
}
void t4(void) {
- size_t size;
- int *p = malloc(sizeof(int));
- scanf("%zu", &size);
- p = (int*) realloc((void*) p, size); // // expected-warning{{realloc is
called with a tainted (potentially attacker controlled) value}}
+ size_t size = 0;
+ int *p = malloc(sizeof(int));
+ scanf("%zu", &size);
+ p = (int*) realloc((void*) p, size); // expected-warning{{realloc is called
with a tainted (potentially attacker controlled) value}}
free(p);
}
void t5(void) {
- size_t size;
- int *p = alloca(sizeof(int));
- scanf("%zu", &size);
- p = (int*) alloca(size); // // expected-warning{{alloca is called with a
tainted (potentially attacker controlled) value}}
+ size_t size = 0;
+ int *p = alloca(sizeof(int));
+ scanf("%zu", &size);
+ p = (int*) alloca(size); // expected-warning{{alloca is called with a
tainted (potentially attacker controlled) value}}
}
diff --git a/clang/test/Analysis/taint-diagnostic-visitor.c
b/clang/test/Analysis/taint-diagnostic-visitor.c
index b8b3710a7013e..dcec6c5236a5c 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=alpha.security.taint,core,alpha.security.ArrayBoundV2
-analyzer-output=text -verify %s
+// RUN: %clang_cc1 -analyze
-analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2,optin.taint.TaintMalloc
-analyzer-output=text -verify %s
// This file is for testing enhanced diagnostics produced by the
GenericTaintChecker
@@ -115,3 +115,13 @@ void multipleTaintedArgs(void) {
system(buf); // expected-warning {{Untrusted data is passed to a system
call}}
// expected-note@-1{{Untrusted data is passed to a system call}}
}
+
+void testTaintedMalloc(){
+ size_t size = 0;
+ scanf("%zu", &size); // expected-note {{Value assigned to 'size'}}
+ // expected-note@-1 {{Taint originated here}}
+ // expected-note@-2 {{Taint propagated to the 2nd
argument}}
+ int *p = malloc(size);// expected-warning{{malloc is called with a tainted
(potentially attacker controlled) value}}
+ // expected-note@-1{{malloc is called with a tainted
(potentially attacker controlled) value}}
+ free(p);
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
