[clang] 26b19a6 - [clang][analyzer]Fix non-effective taint sanitation

[Daniel Krupp via cfe-commits]

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);