boga95 created this revision.
boga95 added reviewers: NoQ, Szelethus, xazax.hun, dkrupp.
Herald added subscribers: cfe-commits, Charusso, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.
Herald added a project: clang.
The `TaintPropagationRule` deduction uses the custom rules from the config.
Check custom sinks when looking for errors. Give an error when at least one of
the specified arguments is tainted.
Emit error message and omit a parameter if it's out of bound.
Repository:
rC Clang
https://reviews.llvm.org/D59637
Files:
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
test/Analysis/Inputs/taint-generic-config.yaml
test/Analysis/taint-generic.c
Index: test/Analysis/taint-generic.c
===================================================================
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
-// RUN: %clang_analyze_cc1 -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1 -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify %s
int scanf(const char *restrict format, ...);
char *gets(char *str);
@@ -295,3 +295,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: test/Analysis/Inputs/taint-generic-config.yaml
===================================================================
--- /dev/null
+++ test/Analysis/Inputs/taint-generic-config.yaml
@@ -0,0 +1,51 @@
+# A list of source/propagation function
+Propagations:
+ # int x = mySource1(); // x is tainted
+ - Name: mySource1
+ DstArgs: [4294967294] # Index for return value
+
+ # int x;
+ # mySource2(&x); // x is tainted
+ - Name: mySource2
+ DstArgs: [0]
+
+ # int x, y;
+ # myScanf("%d %d", &x, &y); // x and y are tainted
+ - Name: myScanf
+ VarType: Dst
+ VarIndex: 1
+
+ # int x; // x is tainted
+ # int y;
+ # myPropagator(x, &y); // y is tainted
+ - Name: myPropagator
+ SrcArgs: [0]
+ DstArgs: [1]
+
+ # const unsigned size = 100;
+ # char buf[size];
+ # int x, y;
+ # int n = mySprintf(buf, size, "%d %d", x, y); // If size, x or y is tainted
+ # // the return value and the buf will be tainted
+ - Name: mySnprintf
+ SrcArgs: [1]
+ DstArgs: [0, 4294967294]
+ VarType: Src
+ VarIndex: 3
+
+# A list of filter functions
+Filters:
+ # int x; // x is tainted
+ # myFilter(&x); // x is not tainted anymore
+ - Name: myFilter
+ Args: [0]
+
+# A list of sink functions
+Sinks:
+ # int x, y; // x and y are tainted
+ # mySink(x, 0, 1); // It will warn
+ # mySink(0, 1, y); // It will warn
+ # mySink(0, x, 1); // It won't warn
+ - Name: mySink
+ Args: [0, 2]
+
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -116,6 +116,11 @@
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 char 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[],
CheckerContext &C) const;
@@ -225,6 +230,9 @@
"(CERT/STR31-C. Guarantee that storage for strings has sufficient space "
"for character data and the null terminator)";
+const char GenericTaintChecker::MsgCustomSink[] =
+ "Untrusted data is passed to a user defined sink";
+
GenericTaintChecker::NameRuleMap GenericTaintChecker::CustomPropagations;
GenericTaintChecker::NameArgMap GenericTaintChecker::CustomFilters;
@@ -251,7 +259,7 @@
static void mapping(IO &IO, TaintConfig::Propagation &Propagation) {
IO.mapRequired("Name", Propagation.Name);
IO.mapOptional("SrcArgs", Propagation.SrcArgs);
- IO.mapRequired("DstArgs", Propagation.DstArgs);
+ IO.mapOptional("DstArgs", Propagation.DstArgs);
IO.mapOptional("VarType", Propagation.VarType,
GenericTaintChecker::VariadicType::None);
IO.mapOptional("VarIndex", Propagation.VarIndex,
@@ -420,6 +428,10 @@
// or smart memory copy:
// - memccpy - copying until hitting a special character.
+ auto It = CustomPropagations.find(Name);
+ if (It != CustomPropagations.end())
+ return It->getValue();
+
return TaintPropagationRule();
}
@@ -527,6 +539,9 @@
if (checkTaintedBufferSize(CE, FDecl, C))
return true;
+ if (checkCustomSinks(CE, Name, C))
+ return true;
+
return false;
}
@@ -563,8 +578,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;
}
@@ -592,8 +611,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);
}
@@ -747,9 +772,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);
@@ -794,6 +819,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) {
const auto *Checker = mgr.registerChecker<GenericTaintChecker>();
StringRef ConfigFile =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
