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

[steakhal]

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:

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}}
  return n;
}

[xazax.hun]

Please add a test as well. Otherwise looks good.

clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

78

I think a comment somewhere why/when do we check only the prefix would be useful.

[Szelethus]

After tests are added and clang-format-diff.py is ran, this would be an amazing patch, thanks!

[steakhal]

• Tests added.
• Clang-format-diff applied.

[steakhal]

Clarified example usage, especially in contrast of eg.: debug.TaintTest checker.

[steakhal]

If this patch is good to go, could someone commit it?
I don't have commit access (yet).

[Szelethus]

In D74131#1884372, @steakhal wrote:

If this patch is good to go, could someone commit it?
I don't have commit access (yet).

I think you can apply for a commit access, you have a history of high quality patches!

clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

78

This isn't done?

427–430

Might as well create a test case for this.

[steakhal]

Declaring the purpose of this checker:

[...]
This would help to reduce the *noise* that the TaintTest debug checker would
introduce and let you focus on the expected-warnings that you really care
about.

Added test to exactly one argument requirement.

[steakhal]

Marking comments done.

[Szelethus]

LGTM, thanks! Feel free to commit as you're ready.

clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

78

The documentation shows plenty of other similar debug functions, I think this patch is fine in this regard.

clang/docs/analyzer/developer-docs/DebugChecks.rst

void foo(int x) {
       clang_analyzer_denote(x, "$x");
       clang_analyzer_express(x + 1); // expected-warning{{$x + 1}}
     }
 
 - ``void clang_analyzer_express(int);``
 
   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
 ==========
 
 The debug.Stats checker collects various information about the analysis of each
 function, such as how many blocks were reached and if the analyzer timed out.
 
 There is also an additional -analyzer-stats flag, which enables various
 statistics within the analyzer engine. Note the Stats checker (which produces at

clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

 //==- ExprInspectionChecker.cpp - Used for regression tests ------*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
+#include "Taint.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/SValExplainer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols,
   void analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext &C) const;
   void analyzerDump(const CallExpr *CE, CheckerContext &C) const;
   void analyzerExplain(const CallExpr *CE, CheckerContext &C) const;
   void analyzerPrintState(const CallExpr *CE, CheckerContext &C) const;
   void analyzerGetExtent(const CallExpr *CE, CheckerContext &C) const;
   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;
 
   ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C) const;
   ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR,
                           ExplodedNode *N) const;
 

 bool ExprInspectionChecker::evalCall(const CallEvent &Call,
                                      CheckerContext &C) const {
   const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
   if (!CE)
     return false;
 
   // 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))
+  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)
+          .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_hashDump",
+                &ExprInspectionChecker::analyzerHashDump)
           .Case("clang_analyzer_denote", &ExprInspectionChecker::analyzerDenote)
-    .Case("clang_analyzer_express", &ExprInspectionChecker::analyzerExpress)
+          .Case("clang_analyzer_express",
+                &ExprInspectionChecker::analyzerExpress)
+          .StartsWith("clang_analyzer_isTainted",
+                      &ExprInspectionChecker::analyzerIsTainted)
           .Default(nullptr);
 
   if (!Handler)
     return false;
 
   (this->*Handler)(CE, C);
   return true;
 }
 
void ExprInspectionChecker::analyzerExpress(const CallExpr *CE,
   if (!Str) {
     reportBug("Unable to express", C);
     return;
   }
 
   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>();
 }
 
 bool ento::shouldRegisterExprInspectionChecker(const LangOptions &LO) {
   return true;
 }

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

+// 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@-1 {{clang_analyzer_isTainted() requires exactly one argument}}
+}