C++: Taint through RangeBasedForStmt (AST only)

Author: jbj

cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll

@@ -65,6 +65,15 @@ predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeT
     // tracking. The flow from expression `x` into `x++` etc. is handled in the
     // case above.
     exprTo = DataFlow::getAnAccessToAssignedVariable(exprFrom.(PostfixCrementOperation))
+    or
+    // In `for (char c : s) { ... c ... }`, this rule propagates taint from `s`
+    // to `c`.
+    exists(RangeBasedForStmt rbf |
+      exprFrom = rbf.getRange() and
+      // It's guaranteed up to at least C++20 that the range-based for loop
+      // desugars to a variable with an initializer.
+      exprTo = rbf.getVariable().getInitializer().getExpr()
+    )
   )
   or
   // Taint can flow through modeled functions

cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

@@ -417,6 +417,7 @@
 | stl.cpp:239:15:239:15 | ref arg (__range) | stl.cpp:239:15:239:15 | (__range) |  |
 | stl.cpp:239:15:239:15 | s | stl.cpp:239:15:239:15 | (__range) |  |
 | stl.cpp:239:15:239:15 | s | stl.cpp:239:15:239:15 | (__range) |  |
+| stl.cpp:239:15:239:15 | s | stl.cpp:239:15:239:15 | call to operator* | TAINT |
 | stl.cpp:243:33:243:33 | ref arg s | stl.cpp:243:50:243:50 | s |  |
 | stl.cpp:243:33:243:33 | ref arg s | stl.cpp:247:16:247:16 | s |  |
 | stl.cpp:243:35:243:39 | call to begin | stl.cpp:243:44:243:45 | it |  |
@@ -438,6 +439,7 @@
 | stl.cpp:247:16:247:16 | ref arg (__range) | stl.cpp:247:16:247:16 | (__range) |  |
 | stl.cpp:247:16:247:16 | s | stl.cpp:247:16:247:16 | (__range) |  |
 | stl.cpp:247:16:247:16 | s | stl.cpp:247:16:247:16 | (__range) |  |
+| stl.cpp:247:16:247:16 | s | stl.cpp:247:16:247:16 | call to operator* | TAINT |
 | stl.cpp:251:28:251:33 | call to source | stl.cpp:251:28:251:36 | call to basic_string | TAINT |
 | stl.cpp:251:28:251:36 | call to basic_string | stl.cpp:252:22:252:28 | const_s |  |
 | stl.cpp:252:22:252:22 | call to begin | stl.cpp:252:22:252:22 | (__begin) |  |
@@ -450,6 +452,7 @@
 | stl.cpp:252:22:252:22 | ref arg (__begin) | stl.cpp:252:22:252:22 | (__begin) |  |
 | stl.cpp:252:22:252:28 | const_s | stl.cpp:252:22:252:22 | (__range) |  |
 | stl.cpp:252:22:252:28 | const_s | stl.cpp:252:22:252:22 | (__range) |  |
+| stl.cpp:252:22:252:28 | const_s | stl.cpp:252:22:252:22 | call to operator* | TAINT |
 | structlikeclass.cpp:5:7:5:7 | Unknown literal | structlikeclass.cpp:5:7:5:7 | constructor init of field v | TAINT |
 | structlikeclass.cpp:5:7:5:7 | Unknown literal | structlikeclass.cpp:5:7:5:7 | constructor init of field v | TAINT |
 | structlikeclass.cpp:5:7:5:7 | this | structlikeclass.cpp:5:7:5:7 | constructor init of field v [pre-this] |  |

cpp/ql/test/library-tests/dataflow/taint-tests/stl.cpp

@@ -237,19 +237,19 @@ void sink(char) {}
 void test_range_based_for_loop() {
 	std::string s(source());
 	for(char c : s) {
-		sink(c); // tainted [NOT DETECTED]
+		sink(c); // tainted [NOT DETECTED by IR]
 	}
 
 	for(std::string::iterator it = s.begin(); it != s.end(); ++it) {
 		sink(*it); // tainted [NOT DETECTED]
 	}
 
 	for(char& c : s) {
-		sink(c); // tainted [NOT DETECTED]
+		sink(c); // tainted [NOT DETECTED by IR]
 	}
 
 	const std::string const_s(source());
 	for(const char& c : const_s) {
-		sink(c); // tainted [NOT DETECTED]
+		sink(c); // tainted [NOT DETECTED by IR]
 	}
 }

cpp/ql/test/library-tests/dataflow/taint-tests/taint.expected

@@ -47,6 +47,9 @@
 | stl.cpp:211:8:211:9 | s3 | stl.cpp:207:8:207:13 | call to source |
 | stl.cpp:230:8:230:9 | s1 | stl.cpp:226:32:226:37 | call to source |
 | stl.cpp:231:8:231:9 | s2 | stl.cpp:228:20:228:25 | call to source |
+| stl.cpp:240:8:240:8 | c | stl.cpp:238:16:238:21 | call to source |
+| stl.cpp:248:8:248:8 | c | stl.cpp:238:16:238:21 | call to source |
+| stl.cpp:253:8:253:8 | c | stl.cpp:251:28:251:33 | call to source |
 | structlikeclass.cpp:35:8:35:9 | s1 | structlikeclass.cpp:29:22:29:27 | call to source |
 | structlikeclass.cpp:36:8:36:9 | s2 | structlikeclass.cpp:30:24:30:29 | call to source |
 | structlikeclass.cpp:37:8:37:9 | s3 | structlikeclass.cpp:29:22:29:27 | call to source |

cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected

@@ -44,6 +44,9 @@
 | stl.cpp:211:8:211:9 | stl.cpp:207:8:207:13 | AST only |
 | stl.cpp:230:8:230:9 | stl.cpp:226:32:226:37 | AST only |
 | stl.cpp:231:8:231:9 | stl.cpp:228:20:228:25 | AST only |
+| stl.cpp:240:8:240:8 | stl.cpp:238:16:238:21 | AST only |
+| stl.cpp:248:8:248:8 | stl.cpp:238:16:238:21 | AST only |
+| stl.cpp:253:8:253:8 | stl.cpp:251:28:251:33 | AST only |
 | structlikeclass.cpp:35:8:35:9 | structlikeclass.cpp:29:22:29:27 | AST only |
 | structlikeclass.cpp:36:8:36:9 | structlikeclass.cpp:30:24:30:29 | AST only |
 | structlikeclass.cpp:37:8:37:9 | structlikeclass.cpp:29:22:29:27 | AST only |