[analyzer] Add bug visitor for taint checker.
Add a bug visitor to the taint checker to make it easy to distinguish where
the tainted value originated. This is especially useful when the original
taint source is obscured by complex data flow.
A patch by Vlad Tsyrklevich!
Differential Revision: https://reviews.llvm.org/D30289
llvm-svn: 297324
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 8c8acc6..af334c4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -101,6 +101,22 @@
bool generateReportIfTainted(const Expr *E, const char Msg[],
CheckerContext &C) const;
+ /// The bug visitor prints a diagnostic message at the location where a given
+ /// variable was tainted.
+ class TaintBugVisitor
+ : public BugReporterVisitorImpl<TaintBugVisitor> {
+ private:
+ const SVal V;
+
+ public:
+ TaintBugVisitor(const SVal V) : V(V) {}
+ void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(V); }
+
+ std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+ };
typedef SmallVector<unsigned, 2> ArgVector;
@@ -194,6 +210,28 @@
/// points to data, which should be tainted on return.
REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned)
+std::shared_ptr<PathDiagnosticPiece>
+GenericTaintChecker::TaintBugVisitor::VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) {
+
+ // Find the ExplodedNode where the taint was first introduced
+ if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V))
+ return nullptr;
+
+ const Stmt *S = PathDiagnosticLocation::getStmt(N);
+ if (!S)
+ return nullptr;
+
+ const LocationContext *NCtx = N->getLocationContext();
+ PathDiagnosticLocation L =
+ PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx);
+ if (!L.isValid() || !L.asLocation().isValid())
+ return nullptr;
+
+ return std::make_shared<PathDiagnosticEventPiece>(
+ L, "Taint originated here");
+}
+
GenericTaintChecker::TaintPropagationRule
GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
const FunctionDecl *FDecl,
@@ -635,8 +673,13 @@
// Check for taint.
ProgramStateRef State = C.getState();
- if (!State->isTainted(getPointedToSymbol(C, E)) &&
- !State->isTainted(E, C.getLocationContext()))
+ const SymbolRef PointedToSym = getPointedToSymbol(C, E);
+ SVal TaintedSVal;
+ if (State->isTainted(PointedToSym))
+ TaintedSVal = nonloc::SymbolVal(PointedToSym);
+ else if (State->isTainted(E, C.getLocationContext()))
+ TaintedSVal = C.getSVal(E);
+ else
return false;
// Generate diagnostic.
@@ -644,6 +687,7 @@
initBugType();
auto report = llvm::make_unique<BugReport>(*BT, Msg, N);
report->addRange(E->getSourceRange());
+ report->addVisitor(llvm::make_unique<TaintBugVisitor>(TaintedSVal));
C.emitReport(std::move(report));
return true;
}
diff --git a/clang/test/Analysis/taint-diagnostic-visitor.c b/clang/test/Analysis/taint-diagnostic-visitor.c
new file mode 100644
index 0000000..50fc0b6
--- /dev/null
+++ b/clang/test/Analysis/taint-diagnostic-visitor.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,core -analyzer-output=text -verify %s
+
+// This file is for testing enhanced diagnostics produced by the GenericTaintChecker
+
+int scanf(const char *restrict format, ...);
+int system(const char *command);
+
+void taintDiagnostic()
+{
+ char buf[128];
+ scanf("%s", buf); // expected-note {{Taint originated here}}
+ system(buf); // expected-warning {{Untrusted data is passed to a system call}} // expected-note {{Untrusted data is passed to a system call (CERT/STR02-C. Sanitize data passed to complex subsystems)}}
+}