[clang] [analyzer] Splitting TaintPropagation checker into reporting and mode… (PR #98157)
Daniel Krupp via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 10 06:00:15 PDT 2024
https://github.com/dkrupp updated https://github.com/llvm/llvm-project/pull/98157
>From 75675417c324a2d1df5e42a8549f6d4bcb779ab4 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Fri, 5 Jul 2024 14:02:00 +0200
Subject: [PATCH 1/4] [analyzer] Splitting TaintPropagation checker into
reporting and modeling checkers
Taint propagation is a a generic modeling feature of the
Clang Static Analyzer which many other checkers depend on.
Therefore GenericTaintChecker is split into a TaintPropagation modeling
checker and a GenericTaint reporting checker.
Other checkers, which report taint related warnings, should
set the TaintPropagation checker as their dependency.
---
clang/docs/analyzer/checkers.rst | 8 ++---
.../user-docs/TaintAnalysisConfiguration.rst | 13 +++++----
.../clang/StaticAnalyzer/Checkers/Checkers.td | 12 +++++---
.../Checkers/GenericTaintChecker.cpp | 29 +++++++++++++++----
4 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 42c097d973d53..9216b1637271d 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1013,7 +1013,7 @@ covers the SEI Cert coding standard rule `INT04-C
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.
+:ref:`alpha-security-taint-GenericTaint` checker for an example.
.. code-block:: c
@@ -3011,10 +3011,10 @@ alpha.security.taint
Checkers implementing
`taint analysis <https://en.wikipedia.org/wiki/Taint_checking>`_.
-.. _alpha-security-taint-TaintPropagation:
+.. _alpha-security-taint-GenericTaint:
-alpha.security.taint.TaintPropagation (C, C++)
-""""""""""""""""""""""""""""""""""""""""""""""
+alpha.security.taint.GenericTaint (C, C++)
+""""""""""""""""""""""""""""""""""""""""""
Taint analysis identifies potential security vulnerabilities where the
attacker can inject malicious data to the program to execute an attack
diff --git a/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst b/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
index 94db84494e00b..bb10b3be758e4 100644
--- a/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
+++ b/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
@@ -2,10 +2,13 @@
Taint Analysis Configuration
============================
-The Clang Static Analyzer uses taint analysis to detect security-related issues in code.
-The backbone of taint analysis in the Clang SA is the `GenericTaintChecker`, which the user can access via the :ref:`alpha-security-taint-TaintPropagation` checker alias and this checker has a default taint-related configuration.
-The built-in default settings are defined in code, and they are always in effect once the checker is enabled, either directly or via the alias.
-The checker also provides a configuration interface for extending the default settings by providing a configuration file in `YAML <http://llvm.org/docs/YamlIO.html#introduction-to-yaml>`_ format.
+The Clang Static Analyzer uses taint analysis to detect injection vulnerability related issues in code.
+The backbone of taint analysis in the Clang SA is the ``TaintPropagation`` modeling checker.
+The reports are emitted via the :ref:`alpha-security-taint-GenericTaint` checker.
+The ``TaintPropagation`` checker has a default taint-related configuration.
+The built-in default settings are defined in code, and they are always in effect.
+The checker also provides a configuration interface for extending the default settings via the ``alpha.security.taint.TaintPropagation:Config`` checker config parameter
+by providing a configuration file to the in `YAML <http://llvm.org/docs/YamlIO.html#introduction-to-yaml>`_ format.
This documentation describes the syntax of the configuration file and gives the informal semantics of the configuration options.
.. contents::
@@ -18,7 +21,7 @@ ________
Taint analysis works by checking for the occurrence of special operations during the symbolic execution of the program.
Taint analysis defines sources, sinks, and propagation rules. It identifies errors by detecting a flow of information that originates from a taint source, reaches a taint sink, and propagates through the program paths via propagation rules.
-A source, sink, or an operation that propagates taint is mainly domain-specific knowledge, but there are some built-in defaults provided by :ref:`alpha-security-taint-TaintPropagation`.
+A source, sink, or an operation that propagates taint is mainly domain-specific knowledge, but there are some built-in defaults provided by ``TaintPropagation`` checker.
It is possible to express that a statement sanitizes tainted values by providing a ``Filters`` section in the external configuration (see :ref:`clangsa-taint-configuration-example` and :ref:`clangsa-taint-filter-details`).
There are no default filters defined in the built-in settings.
The checker's documentation also specifies how to provide a custom taint configuration with command-line options.
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 6e224a4e098ad..ec5dbd28a5272 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1071,7 +1071,7 @@ def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">,
let ParentPackage = Taint in {
-def GenericTaintChecker : Checker<"TaintPropagation">,
+def TaintPropagationChecker : Checker<"TaintPropagation">, // Modelling checker
HelpText<"Generate taint information used by other checkers">,
CheckerOptions<[
CmdLineOption<String,
@@ -1080,6 +1080,12 @@ def GenericTaintChecker : Checker<"TaintPropagation">,
"",
InAlpha>,
]>,
+ Documentation<NotDocumented>,
+ Hidden;
+
+def GenericTaintChecker : Checker<"GenericTaint">,
+ HelpText<"Reports potential injection vulnerabilities">,
+ Dependencies<[TaintPropagationChecker]>,
Documentation<HasDocumentation>;
} // end "alpha.security.taint"
@@ -1717,9 +1723,7 @@ let ParentPackage = TaintOptIn in {
def TaintedAllocChecker: Checker<"TaintedAlloc">,
HelpText<"Check for memory allocations, where the size parameter "
"might be a tainted (attacker controlled) value.">,
- Dependencies<[DynamicMemoryModeling]>,
- //FIXME: GenericTaintChecker should be a dependency, but only after it
- //is transformed into a modeling checker
+ Dependencies<[DynamicMemoryModeling, TaintPropagationChecker]>,
Documentation<HasDocumentation>;
} // end "optin.taint"
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index eb40a4cd3d902..465e7daa18891 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -27,6 +27,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/YAMLTraits.h"
#include <limits>
@@ -391,8 +392,11 @@ class GenericTaintChecker : public Checker<check::PreCall, check::PostCall> {
bool generateReportIfTainted(const Expr *E, StringRef Msg,
CheckerContext &C) const;
+ bool isTaintReporterCheckerEnabled = false;
+ CheckerNameRef reporterCheckerName;
+
private:
- const BugType BT{this, "Use of Untrusted Data", categories::TaintedData};
+ mutable std::unique_ptr<BugType> BT;
bool checkUncontrolledFormatString(const CallEvent &Call,
CheckerContext &C) const;
@@ -1033,6 +1037,8 @@ bool GenericTaintRule::UntrustedEnv(CheckerContext &C) {
bool GenericTaintChecker::generateReportIfTainted(const Expr *E, StringRef Msg,
CheckerContext &C) const {
assert(E);
+ if (!isTaintReporterCheckerEnabled)
+ return false;
std::optional<SVal> TaintedSVal =
getTaintedPointeeOrPointer(C.getState(), C.getSVal(E));
@@ -1040,13 +1046,16 @@ bool GenericTaintChecker::generateReportIfTainted(const Expr *E, StringRef Msg,
return false;
// Generate diagnostic.
- if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
- auto report = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
+ if (!BT)
+ BT.reset(new BugType(reporterCheckerName, "Use of Untrusted Data",
+ categories::TaintedData));
+ static CheckerProgramPointTag Tag(reporterCheckerName, Msg);
+ if (ExplodedNode *N = C.generateNonFatalErrorNode(C.getState(), &Tag)) {
+ auto report = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
report->addRange(E->getSourceRange());
for (auto TaintedSym : getTaintedSymbols(C.getState(), *TaintedSVal)) {
report->markInteresting(TaintedSym);
}
-
C.emitReport(std::move(report));
return true;
}
@@ -1122,10 +1131,20 @@ void GenericTaintChecker::taintUnsafeSocketProtocol(const CallEvent &Call,
}
/// Checker registration
-void ento::registerGenericTaintChecker(CheckerManager &Mgr) {
+void ento::registerTaintPropagationChecker(CheckerManager &Mgr) {
Mgr.registerChecker<GenericTaintChecker>();
}
+bool ento::shouldRegisterTaintPropagationChecker(const CheckerManager &mgr) {
+ return true;
+}
+
+void ento::registerGenericTaintChecker(CheckerManager &Mgr) {
+ GenericTaintChecker *checker = Mgr.getChecker<GenericTaintChecker>();
+ checker->isTaintReporterCheckerEnabled = true;
+ checker->reporterCheckerName = Mgr.getCurrentCheckerName();
+}
+
bool ento::shouldRegisterGenericTaintChecker(const CheckerManager &mgr) {
return true;
}
>From 97e36dbb4269267b1dba959d9720adca1879fb44 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Wed, 10 Jul 2024 14:55:42 +0200
Subject: [PATCH 2/4] Use std::optional for BugType
---
.../StaticAnalyzer/Checkers/GenericTaintChecker.cpp | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 465e7daa18891..2a8b06027d3b0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -393,11 +393,9 @@ class GenericTaintChecker : public Checker<check::PreCall, check::PostCall> {
CheckerContext &C) const;
bool isTaintReporterCheckerEnabled = false;
- CheckerNameRef reporterCheckerName;
+ std::optional<BugType> BT;
private:
- mutable std::unique_ptr<BugType> BT;
-
bool checkUncontrolledFormatString(const CallEvent &Call,
CheckerContext &C) const;
@@ -1046,10 +1044,7 @@ bool GenericTaintChecker::generateReportIfTainted(const Expr *E, StringRef Msg,
return false;
// Generate diagnostic.
- if (!BT)
- BT.reset(new BugType(reporterCheckerName, "Use of Untrusted Data",
- categories::TaintedData));
- static CheckerProgramPointTag Tag(reporterCheckerName, Msg);
+ static CheckerProgramPointTag Tag(BT->getCheckerName(), Msg);
if (ExplodedNode *N = C.generateNonFatalErrorNode(C.getState(), &Tag)) {
auto report = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
report->addRange(E->getSourceRange());
@@ -1142,7 +1137,8 @@ bool ento::shouldRegisterTaintPropagationChecker(const CheckerManager &mgr) {
void ento::registerGenericTaintChecker(CheckerManager &Mgr) {
GenericTaintChecker *checker = Mgr.getChecker<GenericTaintChecker>();
checker->isTaintReporterCheckerEnabled = true;
- checker->reporterCheckerName = Mgr.getCurrentCheckerName();
+ checker->BT.emplace(Mgr.getCurrentCheckerName(), "Use of Untrusted Data",
+ categories::TaintedData);
}
bool ento::shouldRegisterGenericTaintChecker(const CheckerManager &mgr) {
>From cb6b539677e120fc9b9aea0038cccf3735226d22 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at ericsson.com>
Date: Wed, 10 Jul 2024 14:58:02 +0200
Subject: [PATCH 3/4] Remove obsolete note from TaintAlloc documentation
---
clang/docs/analyzer/checkers.rst | 3 ---
1 file changed, 3 deletions(-)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 9216b1637271d..76a9aae170893 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1003,9 +1003,6 @@ array new C++ operator 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
>From 37e50af8678e037b5b8f1645c857d42cbc9f74c6 Mon Sep 17 00:00:00 2001
From: Daniel Krupp <daniel.krupp at gmail.com>
Date: Wed, 10 Jul 2024 15:00:02 +0200
Subject: [PATCH 4/4] Update
clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Donát Nagy <donat.nagy at ericsson.com>
---
clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst b/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
index bb10b3be758e4..67e71d558f2ce 100644
--- a/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
+++ b/clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
@@ -21,7 +21,7 @@ ________
Taint analysis works by checking for the occurrence of special operations during the symbolic execution of the program.
Taint analysis defines sources, sinks, and propagation rules. It identifies errors by detecting a flow of information that originates from a taint source, reaches a taint sink, and propagates through the program paths via propagation rules.
-A source, sink, or an operation that propagates taint is mainly domain-specific knowledge, but there are some built-in defaults provided by ``TaintPropagation`` checker.
+A source, sink, or an operation that propagates taint is mainly domain-specific knowledge, but there are some built-in defaults provided by the ``TaintPropagation`` checker.
It is possible to express that a statement sanitizes tainted values by providing a ``Filters`` section in the external configuration (see :ref:`clangsa-taint-configuration-example` and :ref:`clangsa-taint-filter-details`).
There are no default filters defined in the built-in settings.
The checker's documentation also specifies how to provide a custom taint configuration with command-line options.
More information about the cfe-commits
mailing list