boga95 updated this revision to Diff 215268.
boga95 marked an inline comment as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59637/new/
https://reviews.llvm.org/D59637
Files:
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
test/Analysis/taint-generic.c
Index: test/Analysis/taint-generic.c
===================================================================
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -338,3 +338,43 @@
if (i < rhs)
*(volatile int *) 0; // no-warning
}
+
+
+// Test configuration
+int mySource1();
+void mySource2(int*);
+void myScanf(const char*, ...);
+int myPropagator(int, int*);
+int mySnprintf(char*, size_t, const char*, ...);
+void mySink(int, int, int);
+
+void testConfigurationSources1() {
+ int x = mySource1();
+ Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationSources2() {
+ int x;
+ mySource2(&x);
+ Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationSources3() {
+ int x, y;
+ myScanf("%d %d", &x, &y);
+ Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationPropagation() {
+ int x = mySource1();
+ int y;
+ myPropagator(x, &y);
+ Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationSinks() {
+ int x = mySource1();
+ mySink(x, 1, 2); // expected-warning {{Untrusted data is passed to a user defined sink}}
+ mySink(1, x, 2); // no-warning
+ mySink(1, 2, x); // expected-warning {{Untrusted data is passed to a user defined sink}}
+}
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -115,25 +115,30 @@
static Optional<SVal> getPointedToSVal(CheckerContext &C, const Expr *Arg);
/// Check for CWE-134: Uncontrolled Format String.
- static const char MsgUncontrolledFormatString[];
+ static const llvm::StringLiteral MsgUncontrolledFormatString;
bool checkUncontrolledFormatString(const CallExpr *CE,
CheckerContext &C) const;
/// Check for:
/// CERT/STR02-C. "Sanitize data passed to complex subsystems"
/// CWE-78, "Failure to Sanitize Data into an OS Command"
- static const char MsgSanitizeSystemArgs[];
+ static const llvm::StringLiteral MsgSanitizeSystemArgs;
bool checkSystemCall(const CallExpr *CE, StringRef Name,
CheckerContext &C) const;
/// Check if tainted data is used as a buffer size ins strn.. functions,
/// and allocators.
- static const char MsgTaintedBufferSize[];
+ static const llvm::StringLiteral MsgTaintedBufferSize;
bool checkTaintedBufferSize(const CallExpr *CE, const FunctionDecl *FDecl,
CheckerContext &C) const;
+ /// Check if tainted data is used as a custom sink's parameter.
+ static const llvm::StringLiteral MsgCustomSink;
+ bool checkCustomSinks(const CallExpr *CE, StringRef Name,
+ CheckerContext &C) const;
+
/// Generate a report if the expression is tainted or points to tainted data.
- bool generateReportIfTainted(const Expr *E, const char Msg[],
+ bool generateReportIfTainted(const Expr *E, const StringRef Msg,
CheckerContext &C) const;
/// A struct used to specify taint propagation rules for a function.
@@ -175,7 +180,8 @@
/// Get the propagation rule for a given function.
static TaintPropagationRule
- getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name,
+ getTaintPropagationRule(const GenericTaintChecker *Checker,
+ const FunctionDecl *FDecl, StringRef Name,
CheckerContext &C);
void addSrcArg(unsigned A) { SrcArgs.push_back(A); }
@@ -228,18 +234,22 @@
const unsigned GenericTaintChecker::ReturnValueIndex;
const unsigned GenericTaintChecker::InvalidArgIndex;
-const char GenericTaintChecker::MsgUncontrolledFormatString[] =
+const llvm::StringLiteral GenericTaintChecker::MsgUncontrolledFormatString =
"Untrusted data is used as a format string "
"(CWE-134: Uncontrolled Format String)";
-const char GenericTaintChecker::MsgSanitizeSystemArgs[] =
+const llvm::StringLiteral GenericTaintChecker::MsgSanitizeSystemArgs =
"Untrusted data is passed to a system call "
"(CERT/STR02-C. Sanitize data passed to complex subsystems)";
-const char GenericTaintChecker::MsgTaintedBufferSize[] =
+const llvm::StringLiteral GenericTaintChecker::MsgTaintedBufferSize =
"Untrusted data is used to specify the buffer size "
"(CERT/STR31-C. Guarantee that storage for strings has sufficient space "
"for character data and the null terminator)";
+
+const llvm::StringLiteral GenericTaintChecker::MsgCustomSink =
+ "Untrusted data is passed to a user defined sink";
+;
} // end of anonymous namespace
using TaintConfig = GenericTaintChecker::TaintConfiguration;
@@ -330,7 +340,8 @@
GenericTaintChecker::TaintPropagationRule
GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
- const FunctionDecl *FDecl, StringRef Name, CheckerContext &C) {
+ const GenericTaintChecker *Checker, const FunctionDecl *FDecl,
+ StringRef Name, CheckerContext &C) {
// TODO: Currently, we might lose precision here: we always mark a return
// value as tainted even if it's just a pointer, pointing to tainted data.
@@ -424,6 +435,10 @@
// or smart memory copy:
// - memccpy - copying until hitting a special character.
+ auto It = Checker->CustomPropagations.find(Name);
+ if (It != Checker->CustomPropagations.end())
+ return It->getValue();
+
return TaintPropagationRule();
}
@@ -464,7 +479,7 @@
// First, try generating a propagation rule for this function.
TaintPropagationRule Rule =
- TaintPropagationRule::getTaintPropagationRule(FDecl, Name, C);
+ TaintPropagationRule::getTaintPropagationRule(this, FDecl, Name, C);
if (!Rule.isNull()) {
State = Rule.process(CE, C);
if (!State)
@@ -536,6 +551,9 @@
if (checkTaintedBufferSize(CE, FDecl, C))
return true;
+ if (checkCustomSinks(CE, Name, C))
+ return true;
+
return false;
}
@@ -572,8 +590,12 @@
// Check for taint in arguments.
bool IsTainted = true;
for (unsigned ArgNum : SrcArgs) {
- if (ArgNum >= CE->getNumArgs())
- return State;
+ if (ArgNum >= CE->getNumArgs()) {
+ StringRef Name = C.getCalleeName(C.getCalleeDecl(CE));
+ llvm::errs() << "Skip out of bound SrcArg: " << Name << ":" << ArgNum
+ << '\n';
+ continue;
+ }
if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(ArgNum), State, C)))
break;
}
@@ -601,8 +623,14 @@
continue;
}
+ if (ArgNum >= CE->getNumArgs()) {
+ StringRef Name = C.getCalleeName(C.getCalleeDecl(CE));
+ llvm::errs() << "Skip out of bound DstArg: " << Name << ":" << ArgNum
+ << '\n';
+ continue;
+ }
+
// Mark the given argument.
- assert(ArgNum < CE->getNumArgs());
State = State->add<TaintArgsOnPostVisit>(ArgNum);
}
@@ -700,7 +728,7 @@
}
bool GenericTaintChecker::generateReportIfTainted(const Expr *E,
- const char Msg[],
+ const StringRef Msg,
CheckerContext &C) const {
assert(E);
@@ -756,9 +784,9 @@
.Case("execvP", 0)
.Case("execve", 0)
.Case("dlopen", 0)
- .Default(UINT_MAX);
+ .Default(InvalidArgIndex);
- if (ArgNum == UINT_MAX || CE->getNumArgs() < (ArgNum + 1))
+ if (ArgNum == InvalidArgIndex || CE->getNumArgs() < (ArgNum + 1))
return false;
return generateReportIfTainted(CE->getArg(ArgNum), MsgSanitizeSystemArgs, C);
@@ -803,6 +831,27 @@
generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C);
}
+bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE, StringRef Name,
+ CheckerContext &C) const {
+ auto It = CustomSinks.find(Name);
+ if (It == CustomSinks.end())
+ return false;
+
+ const GenericTaintChecker::ArgVector &Args = It->getValue();
+ for (unsigned ArgNum : Args) {
+ if (ArgNum >= CE->getNumArgs()) {
+ llvm::errs() << "Skip out of bound sink Arg: " << Name << ":" << ArgNum
+ << '\n';
+ continue;
+ }
+
+ if (generateReportIfTainted(CE->getArg(ArgNum), MsgCustomSink, C))
+ return true;
+ }
+
+ return false;
+}
+
void ento::registerGenericTaintChecker(CheckerManager &Mgr) {
auto *Checker = Mgr.registerChecker<GenericTaintChecker>();
std::string Option{"Config"};
@@ -811,7 +860,7 @@
llvm::Optional<TaintConfig> Config =
getConfiguration<TaintConfig>(Mgr, Checker, Option, ConfigFile);
if (Config)
- Checker->parseConfiguration(Mgr, Option, std::move(Config).getValue());
+ Checker->parseConfiguration(Mgr, Option, std::move(Config.getValue()));
}
bool ento::shouldRegisterGenericTaintChecker(const LangOptions &LO) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
