[clang] [analyzer] Splitting TaintPropagation checker into reporting and mode… (PR #98157)

via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 9 06:29:17 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Daniel Krupp (dkrupp)

<details>
<summary>Changes</summary>

…ling 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.


---
Full diff: https://github.com/llvm/llvm-project/pull/98157.diff


4 Files Affected:

- (modified) clang/docs/analyzer/checkers.rst (+3-3) 
- (modified) clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst (+7-4) 
- (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+8-4) 
- (modified) clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (+23-5) 


``````````diff
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 42c097d973d53..b7a3a4ebd927c 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -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..f0117cbfe02ad 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::
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..578db0dcea277 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,10 @@ 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 +1036,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 +1045,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 +1130,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;
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/98157


More information about the cfe-commits mailing list