This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
dkrupp marked an inline comment as done.
Closed by commit rG26b19a67e5c3: [clang][analyzer]Fix non-effective taint
sanitation (authored by dkrupp).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D155848?vs=542619&id=542885#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155848/new/
https://reviews.llvm.org/D155848
Files:
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/test/Analysis/Inputs/taint-generic-config.yaml
clang/test/Analysis/taint-generic.c
Index: clang/test/Analysis/taint-generic.c
===================================================================
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -1010,7 +1010,8 @@
void myScanf(const char*, ...);
int myPropagator(int, int*);
int mySnprintf(char*, size_t, const char*, ...);
-bool isOutOfRange(const int*);
+bool isOutOfRange(const int*); // const filter function
+void sanitizeCmd(char*); // non-const filter function
void mySink(int, int, int);
void testConfigurationSources1(void) {
@@ -1044,6 +1045,19 @@
Buffer[x] = 1; // no-warning
}
+void testConfigurationFilterNonConst(void) {
+ char buffer[1000];
+ myScanf("%s", buffer); // makes buffer tainted
+ system(buffer); // expected-warning {{Untrusted data is passed to a system
call}}
+}
+
+void testConfigurationFilterNonConst2(void) {
+ char buffer[1000];
+ myScanf("%s", buffer); // makes buffer tainted
+ sanitizeCmd(buffer); // removes taintedness
+ system(buffer); // no-warning
+}
+
void testConfigurationSinks(void) {
int x = mySource1();
mySink(x, 1, 2);
Index: clang/test/Analysis/Inputs/taint-generic-config.yaml
===================================================================
--- clang/test/Analysis/Inputs/taint-generic-config.yaml
+++ clang/test/Analysis/Inputs/taint-generic-config.yaml
@@ -69,6 +69,11 @@
Scope: "myAnotherNamespace::"
Args: [0]
+ # char *str; // str is tainted
+ # sanitizeCmd(str) // str is not tainted anymore
+ - Name: sanitizeCmd
+ Args: [0]
+
# A list of sink functions
Sinks:
# int x, y; // x and y are tainted
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -926,7 +926,9 @@
});
/// Check for taint propagation sources.
- /// A rule is relevant if PropSrcArgs is empty, or if any of its signified
+ /// A rule will make the destination variables tainted if PropSrcArgs
+ /// is empty (taints the destination
+ /// arguments unconditionally), or if any of its signified
/// args are tainted in context of the current CallEvent.
bool IsMatching = PropSrcArgs.isEmpty();
std::vector<SymbolRef> TaintedSymbols;
@@ -949,6 +951,8 @@
}
});
+ // Early return for propagation rules which dont match.
+ // Matching propagations, Sinks and Filters will pass this point.
if (!IsMatching)
return;
@@ -975,10 +979,13 @@
Result = F.add(Result, I);
}
+ // Taint property gets lost if the variable is passed as a
+ // non-const pointer or reference to a function which is
+ // not inlined. For matching rules we want to preserve the taintedness.
// TODO: We should traverse all reachable memory regions via the
// escaping parameter. Instead of doing that we simply mark only the
// referred memory region as tainted.
- if (WouldEscape(V, E->getType())) {
+ if (WouldEscape(V, E->getType()) && getTaintedPointeeOrPointer(State,
V)) {
LLVM_DEBUG(if (!Result.contains(I)) {
llvm::dbgs() << "PreCall<";
Call.dump(llvm::dbgs());
Index: clang/test/Analysis/taint-generic.c
===================================================================
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -1010,7 +1010,8 @@
void myScanf(const char*, ...);
int myPropagator(int, int*);
int mySnprintf(char*, size_t, const char*, ...);
-bool isOutOfRange(const int*);
+bool isOutOfRange(const int*); // const filter function
+void sanitizeCmd(char*); // non-const filter function
void mySink(int, int, int);
void testConfigurationSources1(void) {
@@ -1044,6 +1045,19 @@
Buffer[x] = 1; // no-warning
}
+void testConfigurationFilterNonConst(void) {
+ char buffer[1000];
+ myScanf("%s", buffer); // makes buffer tainted
+ system(buffer); // expected-warning {{Untrusted data is passed to a system call}}
+}
+
+void testConfigurationFilterNonConst2(void) {
+ char buffer[1000];
+ myScanf("%s", buffer); // makes buffer tainted
+ sanitizeCmd(buffer); // removes taintedness
+ system(buffer); // no-warning
+}
+
void testConfigurationSinks(void) {
int x = mySource1();
mySink(x, 1, 2);
Index: clang/test/Analysis/Inputs/taint-generic-config.yaml
===================================================================
--- clang/test/Analysis/Inputs/taint-generic-config.yaml
+++ clang/test/Analysis/Inputs/taint-generic-config.yaml
@@ -69,6 +69,11 @@
Scope: "myAnotherNamespace::"
Args: [0]
+ # char *str; // str is tainted
+ # sanitizeCmd(str) // str is not tainted anymore
+ - Name: sanitizeCmd
+ Args: [0]
+
# A list of sink functions
Sinks:
# int x, y; // x and y are tainted
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -926,7 +926,9 @@
});
/// Check for taint propagation sources.
- /// A rule is relevant if PropSrcArgs is empty, or if any of its signified
+ /// A rule will make the destination variables tainted if PropSrcArgs
+ /// is empty (taints the destination
+ /// arguments unconditionally), or if any of its signified
/// args are tainted in context of the current CallEvent.
bool IsMatching = PropSrcArgs.isEmpty();
std::vector<SymbolRef> TaintedSymbols;
@@ -949,6 +951,8 @@
}
});
+ // Early return for propagation rules which dont match.
+ // Matching propagations, Sinks and Filters will pass this point.
if (!IsMatching)
return;
@@ -975,10 +979,13 @@
Result = F.add(Result, I);
}
+ // Taint property gets lost if the variable is passed as a
+ // non-const pointer or reference to a function which is
+ // not inlined. For matching rules we want to preserve the taintedness.
// TODO: We should traverse all reachable memory regions via the
// escaping parameter. Instead of doing that we simply mark only the
// referred memory region as tainted.
- if (WouldEscape(V, E->getType())) {
+ if (WouldEscape(V, E->getType()) && getTaintedPointeeOrPointer(State, V)) {
LLVM_DEBUG(if (!Result.contains(I)) {
llvm::dbgs() << "PreCall<";
Call.dump(llvm::dbgs());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
