]> granicus.if.org Git - clang/commitdiff
[analyzer] Add bug visitor for taint checker.
authorAnna Zaks <ganna@apple.com>
Thu, 9 Mar 2017 00:01:07 +0000 (00:01 +0000)
committerAnna Zaks <ganna@apple.com>
Thu, 9 Mar 2017 00:01:07 +0000 (00:01 +0000)
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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@297324 91177308-0d34-0410-b5e6-96231b3b80d8

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
test/Analysis/taint-diagnostic-visitor.c [new file with mode: 0644]

index 8c8acc637f1fa7feca2451a7a552ec142aed55dc..af334c414367574bc75adb0ff790de1836f5ec70 100644 (file)
@@ -101,6 +101,22 @@ private:
   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 @@ const char GenericTaintChecker::MsgTaintedBufferSize[] =
 /// 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 @@ bool GenericTaintChecker::generateReportIfTainted(const Expr *E,
 
   // 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 @@ bool GenericTaintChecker::generateReportIfTainted(const Expr *E,
     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/test/Analysis/taint-diagnostic-visitor.c b/test/Analysis/taint-diagnostic-visitor.c
new file mode 100644 (file)
index 0000000..50fc0b6
--- /dev/null
@@ -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)}}
+}