[analyzer] Add TaintBugVisitor to the ArrayBoundV2, DivideZero and …

…VLASize.

Summary: Add `TaintBugVisitor` to the ArrayBoundV2, DivideZero, VLASize to be able to indicate where the taint information originated from.

Reviewers: NoQ, george.karpenkov, xazax.hun, a.sidorin

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, cfe-commits, MTC

Differential Revision: https://reviews.llvm.org/D46007

llvm-svn: 331345

clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

@@ -33,8 +33,8 @@ class ArrayBoundCheckerV2 :
 
   enum OOB_Kind { OOB_Precedes, OOB_Excedes, OOB_Tainted };
 
-  void reportOOB(CheckerContext &C, ProgramStateRef errorState,
-                 OOB_Kind kind) const;
+  void reportOOB(CheckerContext &C, ProgramStateRef errorState, OOB_Kind kind,
+                 std::unique_ptr<BugReporterVisitor> Visitor = nullptr) const;
 
 public:
   void checkLocation(SVal l, bool isLoad, const Stmt*S,
@@ -205,8 +205,10 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
 
     // If we are under constrained and the index variables are tainted, report.
     if (state_exceedsUpperBound && state_withinUpperBound) {
-      if (state->isTainted(rawOffset.getByteOffset())) {
-        reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted);
+      SVal ByteOffset = rawOffset.getByteOffset();
+      if (state->isTainted(ByteOffset)) {
+        reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted,
+                  llvm::make_unique<TaintBugVisitor>(ByteOffset));
         return;
       }
     } else if (state_exceedsUpperBound) {
@@ -226,9 +228,9 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
     checkerContext.addTransition(state);
 }
 
-void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext,
-                                    ProgramStateRef errorState,
-                                    OOB_Kind kind) const {
+void ArrayBoundCheckerV2::reportOOB(
+    CheckerContext &checkerContext, ProgramStateRef errorState, OOB_Kind kind,
+    std::unique_ptr<BugReporterVisitor> Visitor) const {
 
   ExplodedNode *errorNode = checkerContext.generateErrorNode(errorState);
   if (!errorNode)
@@ -255,8 +257,9 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext,
     break;
   }
 
-  checkerContext.emitReport(
-      llvm::make_unique<BugReport>(*BT, os.str(), errorNode));
+  auto BR = llvm::make_unique<BugReport>(*BT, os.str(), errorNode);
+  BR->addVisitor(std::move(Visitor));
+  checkerContext.emitReport(std::move(BR));
 }
 
 #ifndef NDEBUG

clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp

@@ -24,22 +24,23 @@ using namespace ento;
 namespace {
 class DivZeroChecker : public Checker< check::PreStmt<BinaryOperator> > {
   mutable std::unique_ptr<BuiltinBug> BT;
-  void reportBug(const char *Msg,
-                 ProgramStateRef StateZero,
-                 CheckerContext &C) const ;
+  void reportBug(const char *Msg, ProgramStateRef StateZero, CheckerContext &C,
+                 std::unique_ptr<BugReporterVisitor> Visitor = nullptr) const;
+
 public:
   void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
 };
 } // end anonymous namespace
 
-void DivZeroChecker::reportBug(const char *Msg,
-                               ProgramStateRef StateZero,
-                               CheckerContext &C) const {
+void DivZeroChecker::reportBug(
+    const char *Msg, 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"));
 
     auto R = llvm::make_unique<BugReport>(*BT, Msg, N);
+    R->addVisitor(std::move(Visitor));
     bugreporter::trackNullOrUndefValue(N, bugreporter::GetDenomExpr(N), *R);
     C.emitReport(std::move(R));
   }
@@ -78,7 +79,8 @@ void DivZeroChecker::checkPreStmt(const BinaryOperator *B,
 
   bool TaintedD = C.getState()->isTainted(*DV);
   if ((stateNotZero && stateZero && TaintedD)) {
-    reportBug("Division by a tainted value, possibly zero", stateZero, C);
+    reportBug("Division by a tainted value, possibly zero", stateZero, C,
+              llvm::make_unique<TaintBugVisitor>(*DV));
     return;
   }
 

clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp

@@ -32,19 +32,18 @@ class VLASizeChecker : public Checker< check::PreStmt<DeclStmt> > {
   mutable std::unique_ptr<BugType> BT;
   enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted, VLA_Negative };
 
-  void reportBug(VLASize_Kind Kind,
-                 const Expr *SizeE,
-                 ProgramStateRef State,
-                 CheckerContext &C) const;
+  void reportBug(VLASize_Kind Kind, const Expr *SizeE, ProgramStateRef State,
+                 CheckerContext &C,
+                 std::unique_ptr<BugReporterVisitor> Visitor = nullptr) const;
+
 public:
   void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
 };
 } // end anonymous namespace
 
-void VLASizeChecker::reportBug(VLASize_Kind Kind,
-                               const Expr *SizeE,
-                               ProgramStateRef State,
-                               CheckerContext &C) const {
+void VLASizeChecker::reportBug(
+    VLASize_Kind Kind, const Expr *SizeE, ProgramStateRef State,
+    CheckerContext &C, std::unique_ptr<BugReporterVisitor> Visitor) const {
   // Generate an error node.
   ExplodedNode *N = C.generateErrorNode(State);
   if (!N)
@@ -73,6 +72,7 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind,
   }
 
   auto report = llvm::make_unique<BugReport>(*BT, os.str(), N);
+  report->addVisitor(std::move(Visitor));
   report->addRange(SizeE->getSourceRange());
   bugreporter::trackNullOrUndefValue(N, SizeE, *report);
   C.emitReport(std::move(report));
@@ -108,7 +108,8 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
 
   // Check if the size is tainted.
   if (state->isTainted(sizeV)) {
-    reportBug(VLA_Tainted, SE, nullptr, C);
+    reportBug(VLA_Tainted, SE, nullptr, C,
+              llvm::make_unique<TaintBugVisitor>(sizeV));
     return;
   }
 

clang/test/Analysis/taint-diagnostic-visitor.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,core -analyzer-output=text -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-output=text -verify %s
 
 // This file is for testing enhanced diagnostics produced by the GenericTaintChecker
 
@@ -11,3 +11,26 @@ void taintDiagnostic()
   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)}}
 }
+
+int taintDiagnosticOutOfBound() {
+  int index;
+  int Array[] = {1, 2, 3, 4, 5};
+  scanf("%d", &index); // expected-note {{Taint originated here}}
+  return Array[index]; // expected-warning {{Out of bound memory access (index is tainted)}}
+                       // expected-note@-1 {{Out of bound memory access (index is tainted)}}
+}
+
+int taintDiagnosticDivZero(int operand) {
+  scanf("%d", &operand); // expected-note {{Value assigned to 'operand'}}
+                         // expected-note@-1 {{Taint originated here}}
+  return 10 / operand; // expected-warning {{Division by a tainted value, possibly zero}}
+                       // expected-note@-1 {{Division by a tainted value, possibly zero}}
+}
+
+void taintDiagnosticVLA() {
+  int x;
+  scanf("%d", &x); // expected-note {{Value assigned to 'x'}}
+                   // expected-note@-1 {{Taint originated here}}
+  int vla[x]; // expected-warning {{Declared variable-length array (VLA) has tainted size}}
+              // expected-note@-1 {{Declared variable-length array (VLA) has tainted size}}
+}