Skip to content

Commit

Permalink
[analyzer][taint] Add isTainted debug expression inspection check
Browse files Browse the repository at this point in the history
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

(cherry picked from commit 859bcf4)
  • Loading branch information
steakhal authored and haoNoQ committed Apr 8, 2020
1 parent 7c846c1 commit 5ecc286
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 20 deletions.
22 changes: 22 additions & 0 deletions clang/docs/analyzer/developer-docs/DebugChecks.rst
Expand Up @@ -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
==========

Expand Down
61 changes: 41 additions & 20 deletions clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -45,6 +46,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;
Expand Down Expand Up @@ -72,26 +74,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;
Expand Down Expand Up @@ -410,6 +420,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>();
}
Expand Down
27 changes: 27 additions & 0 deletions 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@-1 {{clang_analyzer_isTainted() requires exactly one argument}}
}

0 comments on commit 5ecc286

Please sign in to comment.