[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.

Author: dkrupp

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

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::

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"

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,20 +1036,25 @@ 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));
 
   if (!TaintedSVal)
     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;
 }