[clang] 859bcf4 - [analyzer][taint] Add isTainted debug expression inspection check

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 3 05:40:34 PST 2020


Author: Balazs Benics
Date: 2020-03-03T14:40:23+01:00
New Revision: 859bcf4e3bb991a161821129d19d50ba00f9c56a

URL: https://github.com/llvm/llvm-project/commit/859bcf4e3bb991a161821129d19d50ba00f9c56a
DIFF: https://github.com/llvm/llvm-project/commit/859bcf4e3bb991a161821129d19d50ba00f9c56a.diff

LOG: [analyzer][taint] Add isTainted debug expression inspection check

Summary:
This patch introduces the `clang_analyzer_isTainted` expression inspection
check for checking taint.

Using this we could query the analyzer whether the expression used as the
argument is tainted or not. This would be useful in tests, where we don't want
to issue warning for all tainted expressions in a given file
(like the `debug.TaintTest` would do) but only for certain expressions.

Example usage:

```lang=c++
int read_integer() {
  int n;
  clang_analyzer_isTainted(n);     // expected-warning{{NO}}
  scanf("%d", &n);
  clang_analyzer_isTainted(n);     // expected-warning{{YES}}
  clang_analyzer_isTainted(n + 2); // expected-warning{{YES}}
  clang_analyzer_isTainted(n > 0); // expected-warning{{YES}}
  int next_tainted_value = n; // no-warning
  return n;
}
```

Reviewers: NoQ, Szelethus, baloghadamsoftware, xazax.hun, boga95

Reviewed By: Szelethus

Subscribers: martong, rnkovacs, whisperity, xazax.hun,
baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, donat.nagy,
Charusso, cfe-commits, boga95, dkrupp, cfe-commits

Tags: #clang

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

Added: 
    clang/test/Analysis/debug-exprinspection-istainted.c

Modified: 
    clang/docs/analyzer/developer-docs/DebugChecks.rst
    clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/analyzer/developer-docs/DebugChecks.rst b/clang/docs/analyzer/developer-docs/DebugChecks.rst
index 3f9bed78604f..05b3e2480d3b 100644
--- a/clang/docs/analyzer/developer-docs/DebugChecks.rst
+++ b/clang/docs/analyzer/developer-docs/DebugChecks.rst
@@ -275,6 +275,28 @@ ExprInspection checks
 
   See clang_analyzer_denote().
 
+- ``void clang_analyzer_isTainted(a single argument of any type);``
+
+  Queries the analyzer whether the expression used as argument is tainted or not.
+  This is useful in tests, where we don't want to issue warning for all tainted
+  expressions but only check for certain expressions.
+  This would help to reduce the *noise* that the `TaintTest` debug checker would
+  introduce and let you focus on the `expected-warning`s that you really care
+  about.
+
+  Example usage::
+
+    int read_integer() {
+      int n;
+      clang_analyzer_isTainted(n);     // expected-warning{{NO}}
+      scanf("%d", &n);
+      clang_analyzer_isTainted(n);     // expected-warning{{YES}}
+      clang_analyzer_isTainted(n + 2); // expected-warning{{YES}}
+      clang_analyzer_isTainted(n > 0); // expected-warning{{YES}}
+      int next_tainted_value = n; // no-warning
+      return n;
+    }
+
 Statistics
 ==========
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 54b364f38f81..10b27831d89f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Taint.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/SValExplainer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -46,6 +47,7 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols,
   void analyzerHashDump(const CallExpr *CE, CheckerContext &C) const;
   void analyzerDenote(const CallExpr *CE, CheckerContext &C) const;
   void analyzerExpress(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerIsTainted(const CallExpr *CE, CheckerContext &C) const;
 
   typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *,
                                                  CheckerContext &C) const;
@@ -73,26 +75,34 @@ bool ExprInspectionChecker::evalCall(const CallEvent &Call,
 
   // These checks should have no effect on the surrounding environment
   // (globals should not be invalidated, etc), hence the use of evalCall.
-  FnCheck Handler = llvm::StringSwitch<FnCheck>(C.getCalleeName(CE))
-    .Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval)
-    .Case("clang_analyzer_checkInlined",
-          &ExprInspectionChecker::analyzerCheckInlined)
-    .Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
-    .Case("clang_analyzer_warnIfReached",
-          &ExprInspectionChecker::analyzerWarnIfReached)
-    .Case("clang_analyzer_warnOnDeadSymbol",
-          &ExprInspectionChecker::analyzerWarnOnDeadSymbol)
-    .StartsWith("clang_analyzer_explain", &ExprInspectionChecker::analyzerExplain)
-    .StartsWith("clang_analyzer_dump", &ExprInspectionChecker::analyzerDump)
-    .Case("clang_analyzer_getExtent", &ExprInspectionChecker::analyzerGetExtent)
-    .Case("clang_analyzer_printState",
-          &ExprInspectionChecker::analyzerPrintState)
-    .Case("clang_analyzer_numTimesReached",
-          &ExprInspectionChecker::analyzerNumTimesReached)
-    .Case("clang_analyzer_hashDump", &ExprInspectionChecker::analyzerHashDump)
-    .Case("clang_analyzer_denote", &ExprInspectionChecker::analyzerDenote)
-    .Case("clang_analyzer_express", &ExprInspectionChecker::analyzerExpress)
-    .Default(nullptr);
+  FnCheck Handler =
+      llvm::StringSwitch<FnCheck>(C.getCalleeName(CE))
+          .Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval)
+          .Case("clang_analyzer_checkInlined",
+                &ExprInspectionChecker::analyzerCheckInlined)
+          .Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
+          .Case("clang_analyzer_warnIfReached",
+                &ExprInspectionChecker::analyzerWarnIfReached)
+          .Case("clang_analyzer_warnOnDeadSymbol",
+                &ExprInspectionChecker::analyzerWarnOnDeadSymbol)
+          .StartsWith("clang_analyzer_explain",
+                      &ExprInspectionChecker::analyzerExplain)
+          .StartsWith("clang_analyzer_dump",
+                      &ExprInspectionChecker::analyzerDump)
+          .Case("clang_analyzer_getExtent",
+                &ExprInspectionChecker::analyzerGetExtent)
+          .Case("clang_analyzer_printState",
+                &ExprInspectionChecker::analyzerPrintState)
+          .Case("clang_analyzer_numTimesReached",
+                &ExprInspectionChecker::analyzerNumTimesReached)
+          .Case("clang_analyzer_hashDump",
+                &ExprInspectionChecker::analyzerHashDump)
+          .Case("clang_analyzer_denote", &ExprInspectionChecker::analyzerDenote)
+          .Case("clang_analyzer_express",
+                &ExprInspectionChecker::analyzerExpress)
+          .StartsWith("clang_analyzer_isTainted",
+                      &ExprInspectionChecker::analyzerIsTainted)
+          .Default(nullptr);
 
   if (!Handler)
     return false;
@@ -412,6 +422,17 @@ void ExprInspectionChecker::analyzerExpress(const CallExpr *CE,
   reportBug(*Str, C);
 }
 
+void ExprInspectionChecker::analyzerIsTainted(const CallExpr *CE,
+                                              CheckerContext &C) const {
+  if (CE->getNumArgs() != 1) {
+    reportBug("clang_analyzer_isTainted() requires exactly one argument", C);
+    return;
+  }
+  const bool IsTainted =
+      taint::isTainted(C.getState(), CE->getArg(0), C.getLocationContext());
+  reportBug(IsTainted ? "YES" : "NO", C);
+}
+
 void ento::registerExprInspectionChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<ExprInspectionChecker>();
 }

diff  --git a/clang/test/Analysis/debug-exprinspection-istainted.c b/clang/test/Analysis/debug-exprinspection-istainted.c
new file mode 100644
index 000000000000..e2f6821e4aa9
--- /dev/null
+++ b/clang/test/Analysis/debug-exprinspection-istainted.c
@@ -0,0 +1,27 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-checker=alpha.security.taint
+
+int scanf(const char *restrict format, ...);
+void clang_analyzer_isTainted(char);
+void clang_analyzer_isTainted_any_suffix(char);
+void clang_analyzer_isTainted_many_arguments(char, int, int);
+
+void foo() {
+  char buf[32] = "";
+  clang_analyzer_isTainted(buf[0]);            // expected-warning {{NO}}
+  clang_analyzer_isTainted_any_suffix(buf[0]); // expected-warning {{NO}}
+  scanf("%s", buf);
+  clang_analyzer_isTainted(buf[0]);            // expected-warning {{YES}}
+  clang_analyzer_isTainted_any_suffix(buf[0]); // expected-warning {{YES}}
+
+  int tainted_value = buf[0]; // no-warning
+}
+
+void exactly_one_argument_required() {
+  char buf[32] = "";
+  scanf("%s", buf);
+  clang_analyzer_isTainted_many_arguments(buf[0], 42, 42);
+  // expected-warning at -1 {{clang_analyzer_isTainted() requires exactly one argument}}
+}


        


More information about the cfe-commits mailing list