Taint source is correctly shown

The alpha.security.taint.TaintPropagation checker
indicated incorrectly the origin of the source if
the taintedness propagated through multiple variables.

This is fixed now.

Author: dkrupp

clang/include/clang/StaticAnalyzer/Checkers/Taint.h

@@ -13,7 +13,9 @@
 #ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_TAINT_H
 #define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_TAINT_H
 
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 
 namespace clang {
@@ -23,27 +25,48 @@ namespace taint {
 /// The type of taint, which helps to differentiate between different types of
 /// taint.
 using TaintTagType = unsigned;
+using TaintFlowType = int;
 
 static constexpr TaintTagType TaintTagGeneric = 0;
+static constexpr TaintFlowType UninitializedFlow = -1;
+
+/// Used to store the taint flow related information
+/// in the program state.
+struct TaintData {
+  TaintTagType Type; // Type of the taintedness (e.g. sql escape checked)
+  TaintFlowType Flow; // Unique identifier of the taint flow
+  TaintData() : Type(TaintTagGeneric), Flow(UninitializedFlow){};
+  TaintData(TaintFlowType f) : Type(TaintTagGeneric), Flow(f){};
+  TaintData(TaintTagType t, TaintFlowType f) : Type(t), Flow(f){};
+  bool operator==(const TaintData &X) const { return Flow == X.Flow; }
+  int operator<(const TaintData &X) const { return Flow < X.Flow; }
+  void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(Flow); }
+};
+
+static inline raw_ostream &operator<<(llvm::raw_ostream &Out,
+                                      const clang::ento::taint::TaintData t) {
+  Out << "Type:" << t.Type << " Flow:" << t.Flow << "\n";
+  return Out;
+}
 
 /// Create a new state in which the value of the statement is marked as tainted.
 [[nodiscard]] ProgramStateRef addTaint(ProgramStateRef State, const Stmt *S,
                                        const LocationContext *LCtx,
-                                       TaintTagType Kind = TaintTagGeneric);
+                                       TaintData Data = TaintData());
 
 /// Create a new state in which the value is marked as tainted.
 [[nodiscard]] ProgramStateRef addTaint(ProgramStateRef State, SVal V,
-                                       TaintTagType Kind = TaintTagGeneric);
+                                       TaintData Data = TaintData());
 
 /// Create a new state in which the symbol is marked as tainted.
 [[nodiscard]] ProgramStateRef addTaint(ProgramStateRef State, SymbolRef Sym,
-                                       TaintTagType Kind = TaintTagGeneric);
+                                       TaintData Data = TaintData());
 
 /// Create a new state in which the pointer represented by the region
 /// is marked as tainted.
 [[nodiscard]] ProgramStateRef addTaint(ProgramStateRef State,
                                        const MemRegion *R,
-                                       TaintTagType Kind = TaintTagGeneric);
+                                       TaintData Data = TaintData());
 
 [[nodiscard]] ProgramStateRef removeTaint(ProgramStateRef State, SVal V);
 
@@ -56,10 +79,38 @@ static constexpr TaintTagType TaintTagGeneric = 0;
 /// This might be necessary when referring to regions that can not have an
 /// individual symbol, e.g. if they are represented by the default binding of
 /// a LazyCompoundVal.
-[[nodiscard]] ProgramStateRef
-addPartialTaint(ProgramStateRef State, SymbolRef ParentSym,
-                const SubRegion *SubRegion,
-                TaintTagType Kind = TaintTagGeneric);
+[[nodiscard]] ProgramStateRef addPartialTaint(ProgramStateRef State,
+                                              SymbolRef ParentSym,
+                                              const SubRegion *SubRegion,
+                                              TaintData = TaintData());
+
+std::optional<TaintData> getTaintData(ProgramStateRef State, const Stmt *S,
+                                 const LocationContext *LCtx);
+
+/// Get the taint data for a value if tainted in the given state.
+std::optional<TaintData> getTaintData(ProgramStateRef State, SVal V);
+
+/// Get the taint data for symbol if tainted in the given state.
+std::optional<TaintData> getTaintData(ProgramStateRef State, SymbolRef Sym);
+
+/// Get the Taint data for a pointer represented by the region
+// if tainted in the given state.
+std::optional<TaintData> getTaintData(ProgramStateRef State, const MemRegion *Reg);
+
+/// Check if the region the expression evaluates to is the standard input,
+/// and thus, is tainted.
+bool isStdin(SVal Val, const ASTContext &ACtx);
+
+/// Returns the TaintData for a pointed value (if tainted), otherwise
+/// the TaintData of the pointer itself.
+std::optional<TaintData> getTaintDataPointeeOrPointer(const CheckerContext &C,
+                                                 SVal Arg);
+std::optional<TaintData> getTaintDataPointeeOrPointer(const Expr *E,
+                                                 const ProgramStateRef &State,
+                                                 CheckerContext &C);
+
+SVal getPointeeOf(const CheckerContext &C, Loc LValue);
+std::optional<SVal> getPointeeOf(const CheckerContext &C, SVal Arg);
 
 /// Check if the statement has a tainted value in the given state.
 bool isTainted(ProgramStateRef State, const Stmt *S,
@@ -84,19 +135,19 @@ void printTaint(ProgramStateRef State, raw_ostream &Out, const char *nl = "\n",
 
 LLVM_DUMP_METHOD void dumpTaint(ProgramStateRef State);
 
-/// The bug visitor prints a diagnostic message at the location where a given
-/// variable was tainted.
-class TaintBugVisitor final : public BugReporterVisitor {
-private:
-  const SVal V;
-
+/// Always use TaintBugReport to give taint related warnings in any checker.
+/// The TaintData member carries the flow associated data of the tainted SVal
+class TaintBugReport : public clang::ento::PathSensitiveBugReport {
 public:
-  TaintBugVisitor(const SVal V) : V(V) {}
-  void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(V); }
-
-  PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
-                                   BugReporterContext &BRC,
-                                   PathSensitiveBugReport &BR) override;
+  TaintBugReport(const BugType &bt, StringRef desc,
+                 const ExplodedNode *errorNode, TaintData T)
+      : clang::ento::PathSensitiveBugReport(bt, desc, errorNode,
+                                            Kind::TaintPathSensitive),
+        taint_data(T){};
+  static bool classof(const BugReport *R) {
+    return R->getKind() == Kind::TaintPathSensitive;
+  }
+  TaintData taint_data;
 };
 
 } // namespace taint

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

@@ -118,7 +118,7 @@ class StackHintGeneratorForSymbol : public StackHintGenerator {
 /// individual bug reports.
 class BugReport {
 public:
-  enum class Kind { Basic, PathSensitive };
+  enum class Kind { Basic, PathSensitive, TaintPathSensitive };
 
 protected:
   friend class BugReportEquivClass;
@@ -367,14 +367,16 @@ class PathSensitiveBugReport : public BugReport {
 
 public:
   PathSensitiveBugReport(const BugType &bt, StringRef desc,
-                         const ExplodedNode *errorNode)
-      : PathSensitiveBugReport(bt, desc, desc, errorNode) {}
+                         const ExplodedNode *errorNode,
+                         BugReport::Kind kind = Kind::PathSensitive)
+      : PathSensitiveBugReport(bt, desc, desc, errorNode, kind) {}
 
   PathSensitiveBugReport(const BugType &bt, StringRef shortDesc, StringRef desc,
-                         const ExplodedNode *errorNode)
+                         const ExplodedNode *errorNode,
+                         BugReport::Kind kind = Kind::PathSensitive)
       : PathSensitiveBugReport(bt, shortDesc, desc, errorNode,
                                /*LocationToUnique*/ {},
-                               /*DeclToUnique*/ nullptr) {}
+                               /*DeclToUnique*/ nullptr, kind) {}
 
   /// Create a PathSensitiveBugReport with a custom uniqueing location.
   ///
@@ -386,17 +388,20 @@ class PathSensitiveBugReport : public BugReport {
   PathSensitiveBugReport(const BugType &bt, StringRef desc,
                          const ExplodedNode *errorNode,
                          PathDiagnosticLocation LocationToUnique,
-                         const Decl *DeclToUnique)
+                         const Decl *DeclToUnique,
+                         BugReport::Kind kind = Kind::PathSensitive)
       : PathSensitiveBugReport(bt, desc, desc, errorNode, LocationToUnique,
-                               DeclToUnique) {}
+                               DeclToUnique, kind) {}
 
   PathSensitiveBugReport(const BugType &bt, StringRef shortDesc, StringRef desc,
                          const ExplodedNode *errorNode,
                          PathDiagnosticLocation LocationToUnique,
-                         const Decl *DeclToUnique);
+                         const Decl *DeclToUnique,
+                         BugReport::Kind kind = Kind::PathSensitive);
 
   static bool classof(const BugReport *R) {
-    return R->getKind() == Kind::PathSensitive;
+    return R->getKind() >= Kind::PathSensitive &&
+           R->getKind() <= Kind::TaintPathSensitive;
   }
 
   const ExplodedNode *getErrorNode() const { return ErrorNode; }

clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h

@@ -22,6 +22,7 @@ extern const char *const CXXObjectLifecycle;
 extern const char *const CXXMoveSemantics;
 extern const char *const SecurityError;
 extern const char *const UnusedCode;
+extern const char *const TaintedData;
 } // namespace categories
 } // namespace ento
 } // namespace clang

clang/include/clang/StaticAnalyzer/Core/BugReporter/TaintBugType.h

@@ -32,12 +32,12 @@ using namespace taint;
 namespace {
 class ArrayBoundCheckerV2 :
     public Checker<check::Location> {
-  mutable std::unique_ptr<BuiltinBug> BT;
+  mutable std::unique_ptr<BugType> BT;
 
   enum OOB_Kind { OOB_Precedes, OOB_Excedes, OOB_Tainted };
 
   void reportOOB(CheckerContext &C, ProgramStateRef errorState, OOB_Kind kind,
-                 std::unique_ptr<BugReporterVisitor> Visitor = nullptr) const;
+                 SVal &Offset) const;
 
 public:
   void checkLocation(SVal l, bool isLoad, const Stmt*S,
@@ -166,7 +166,8 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
 
     // Are we constrained enough to definitely precede the lower bound?
     if (state_precedesLowerBound && !state_withinLowerBound) {
-      reportOOB(checkerContext, state_precedesLowerBound, OOB_Precedes);
+      reportOOB(checkerContext, state_precedesLowerBound, OOB_Precedes,
+                rawOffsetVal);
       return;
     }
 
@@ -208,14 +209,15 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
       SVal ByteOffset = rawOffset.getByteOffset();
       if (isTainted(state, ByteOffset)) {
         reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted,
-                  std::make_unique<TaintBugVisitor>(ByteOffset));
+                  rawOffsetVal);
         return;
       }
     } else if (state_exceedsUpperBound) {
       // If we are constrained enough to definitely exceed the upper bound,
       // report.
       assert(!state_withinUpperBound);
-      reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes);
+      reportOOB(checkerContext, state_exceedsUpperBound, OOB_Excedes,
+                rawOffsetVal);
       return;
     }
 
@@ -227,16 +229,22 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
   checkerContext.addTransition(state);
 }
 
-void ArrayBoundCheckerV2::reportOOB(
-    CheckerContext &checkerContext, ProgramStateRef errorState, OOB_Kind kind,
-    std::unique_ptr<BugReporterVisitor> Visitor) const {
+void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext,
+                                    ProgramStateRef errorState, OOB_Kind kind,
+                                    SVal &Offset) const {
 
   ExplodedNode *errorNode = checkerContext.generateErrorNode(errorState);
   if (!errorNode)
     return;
 
-  if (!BT)
-    BT.reset(new BuiltinBug(this, "Out-of-bound access"));
+  if (!BT) {
+    if (kind != OOB_Tainted)
+      BT.reset(
+          new BugType(this, "Out-of-bound access", categories::LogicError));
+    else
+      BT.reset(
+          new BugType(this, "Out-of-bound access", categories::TaintedData));
+  }
 
   // FIXME: This diagnostics are preliminary.  We should get far better
   // diagnostics for explaining buffer overruns.
@@ -255,9 +263,13 @@ void ArrayBoundCheckerV2::reportOOB(
     os << "(index is tainted)";
     break;
   }
+  std::optional<TaintData> TD = std::nullopt;
+  if (kind == OOB_Tainted)
+    TD = getTaintDataPointeeOrPointer(checkerContext, Offset);
 
-  auto BR = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), errorNode);
-  BR->addVisitor(std::move(Visitor));
+  auto BR =
+      TD ? std::make_unique<TaintBugReport>(*BT, os.str(), errorNode, *TD)
+         : std::make_unique<PathSensitiveBugReport>(*BT, os.str(), errorNode);
   checkerContext.emitReport(std::move(BR));
 }
 

clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

@@ -25,8 +25,9 @@ using namespace taint;
 
 namespace {
 class DivZeroChecker : public Checker< check::PreStmt<BinaryOperator> > {
-  mutable std::unique_ptr<BuiltinBug> BT;
-  void reportBug(const char *Msg, ProgramStateRef StateZero, CheckerContext &C,
+  mutable std::unique_ptr<BugType> BT;
+  void reportBug(const char *Msg, std::string Category,
+                 ProgramStateRef StateZero, CheckerContext &C,
                  std::unique_ptr<BugReporterVisitor> Visitor = nullptr) const;
 
 public:
@@ -42,13 +43,19 @@ static const Expr *getDenomExpr(const ExplodedNode *N) {
 }
 
 void DivZeroChecker::reportBug(
-    const char *Msg, ProgramStateRef StateZero, CheckerContext &C,
-    std::unique_ptr<BugReporterVisitor> Visitor) const {
+    const char *Msg, std::string Category, ProgramStateRef StateZero,
+    CheckerContext &C, std::unique_ptr<BugReporterVisitor> Visitor) const {
   if (ExplodedNode *N = C.generateErrorNode(StateZero)) {
     if (!BT)
-      BT.reset(new BuiltinBug(this, "Division by zero"));
+      BT.reset(new BugType(this, "Division by zero", Category));
 
-    auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+    std::optional<TaintData> TD = std::nullopt;
+    if (Category == categories::TaintedData)
+      TD = getTaintDataPointeeOrPointer(C, C.getSVal(getDenomExpr(N)));
+
+    auto R = TD ? std::make_unique<TaintBugReport>(*BT, Msg, N, *TD)
+                : std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
+    std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
     R->addVisitor(std::move(Visitor));
     bugreporter::trackExpressionValue(N, getDenomExpr(N), *R);
     C.emitReport(std::move(R));
@@ -82,14 +89,14 @@ void DivZeroChecker::checkPreStmt(const BinaryOperator *B,
 
   if (!stateNotZero) {
     assert(stateZero);
-    reportBug("Division by zero", stateZero, C);
+    reportBug("Division by zero", categories::LogicError, stateZero, C);
     return;
   }
 
   bool TaintedD = isTainted(C.getState(), *DV);
   if ((stateNotZero && stateZero && TaintedD)) {
-    reportBug("Division by a tainted value, possibly zero", stateZero, C,
-              std::make_unique<taint::TaintBugVisitor>(*DV));
+    reportBug("Division by a tainted value, possibly zero",
+              categories::TaintedData, stateZero, C);
     return;
   }