Author: Daniel Krupp Date: 2023-07-21T15:11:13+02:00 New Revision: 26b19a67e5c398a30b26214544878ec364dc59af
URL: https://github.com/llvm/llvm-project/commit/26b19a67e5c398a30b26214544878ec364dc59af DIFF: https://github.com/llvm/llvm-project/commit/26b19a67e5c398a30b26214544878ec364dc59af.diff LOG: [clang][analyzer]Fix non-effective taint sanitation There was a bug in alpha.security.taint.TaintPropagation checker in Clang Static Analyzer. Taint filtering could only sanitize const arguments. After this patch, taint filtering is effective also on non-const parameters. Differential Revision: https://reviews.llvm.org/D155848 Added: Modified: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp clang/test/Analysis/Inputs/taint-generic-config.yaml clang/test/Analysis/taint-generic.c Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 4bf8ce13e0572c..3dcb45c0b11038 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -926,7 +926,9 @@ void GenericTaintRule::process(const GenericTaintChecker &Checker, }); /// 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 @@ void GenericTaintRule::process(const GenericTaintChecker &Checker, } }); + // Early return for propagation rules which dont match. + // Matching propagations, Sinks and Filters will pass this point. if (!IsMatching) return; @@ -975,10 +979,13 @@ void GenericTaintRule::process(const GenericTaintChecker &Checker, 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()); diff --git a/clang/test/Analysis/Inputs/taint-generic-config.yaml b/clang/test/Analysis/Inputs/taint-generic-config.yaml index 39b52ccc32e67c..d025bb2a1ed3c8 100755 --- a/clang/test/Analysis/Inputs/taint-generic-config.yaml +++ b/clang/test/Analysis/Inputs/taint-generic-config.yaml @@ -69,6 +69,11 @@ Filters: 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 diff --git a/clang/test/Analysis/taint-generic.c b/clang/test/Analysis/taint-generic.c index 62e1f570b6622a..84b7cc51dd6df8 100644 --- a/clang/test/Analysis/taint-generic.c +++ b/clang/test/Analysis/taint-generic.c @@ -1010,7 +1010,8 @@ void mySource2(int*); 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 @@ void testConfigurationFilter(void) { 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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits