Rust: Handle arrays in taint tracking

Author: paldepind

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -626,7 +626,7 @@ private class StructFieldContent extends Content, TStructFieldContent {
 /**
  * Content stored at an element in an array.
  */
-final private class ArrayElementContent extends VariantContent, TArrayElement {
+final class ArrayElementContent extends Content, TArrayElement {
   ArrayElementContent() { this = TArrayElement() }
 
   override string toString() { result = "array[]" }
@@ -665,7 +665,7 @@ abstract class ContentSet extends TContentSet {
   abstract Content getAReadContent();
 }
 
-final private class SingletonContentSet extends ContentSet, TSingletonContentSet {
+final class SingletonContentSet extends ContentSet, TSingletonContentSet {
   private Content c;
 
   SingletonContentSet() { this = TSingletonContentSet(c) }

rust/ql/lib/codeql/rust/dataflow/internal/TaintTrackingImpl.qll

@@ -1,10 +1,9 @@
 private import rust
 private import codeql.dataflow.TaintTracking
 private import codeql.rust.controlflow.CfgNodes
-private import DataFlowImpl
 private import codeql.rust.dataflow.FlowSummary
-private import FlowSummaryImpl as FlowSummaryImpl
 private import DataFlowImpl
+private import FlowSummaryImpl as FlowSummaryImpl
 
 module RustTaintTracking implements InputSig<Location, RustDataFlow> {
   predicate defaultTaintSanitizer(Node::Node node) { none() }
@@ -35,6 +34,15 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
         pred.asExpr() = index.getBase() and
         succ.asExpr() = index
       )
+      or
+      // Although data flow through collections is modeled using stores/reads,
+      // we also allow taint to flow out of a tainted collection. This is
+      // needed in order to support taint-tracking configurations where the
+      // source is a collection.
+      exists(ContentSet cs |
+        RustDataFlow::readStep(pred, cs, succ) and
+        cs.(SingletonContentSet).getContent() instanceof ArrayElementContent
+      )
     )
     or
     FlowSummaryImpl::Private::Steps::summaryLocalStep(pred.(Node::FlowSummaryNode).getSummaryNode(),
@@ -46,7 +54,10 @@ module RustTaintTracking implements InputSig<Location, RustDataFlow> {
    * and inputs to additional taint steps.
    */
   bindingset[node]
-  predicate defaultImplicitTaintRead(Node::Node node, ContentSet c) { none() }
+  predicate defaultImplicitTaintRead(Node::Node node, ContentSet cs) {
+    exists(node) and
+    cs.(SingletonContentSet).getContent() instanceof ArrayElementContent
+  }
 
   /**
    * Holds if the additional step from `src` to `sink` should be considered in

rust/ql/test/library-tests/dataflow/taint/TaintFlowStep.expected

@@ -6,3 +6,6 @@
 | main.rs:23:13:23:13 | a | main.rs:23:13:23:19 | a as u8 |  |
 | main.rs:24:10:24:10 | b | main.rs:24:10:24:17 | b as i64 |  |
 | main.rs:38:23:38:23 | s | main.rs:38:23:38:29 | s[...] |  |
+| main.rs:54:14:54:16 | arr | main.rs:54:14:54:19 | arr[1] |  |
+| main.rs:64:24:64:24 | s | main.rs:64:24:64:27 | s[1] |  |
+| main.rs:69:9:69:12 | arr2 | main.rs:69:9:69:15 | arr2[1] |  |

rust/ql/test/library-tests/dataflow/taint/inline-taint-flow.expected

@@ -3,16 +3,26 @@ edges
 | main.rs:12:13:12:22 | source(...) | main.rs:13:10:13:14 | ... + ... | provenance |  |
 | main.rs:17:13:17:22 | source(...) | main.rs:18:10:18:11 | - ... | provenance |  |
 | main.rs:22:13:22:22 | source(...) | main.rs:24:10:24:17 | b as i64 | provenance |  |
+| main.rs:53:19:53:28 | source(...) | main.rs:54:14:54:19 | arr[1] | provenance |  |
+| main.rs:69:9:69:12 | [post] arr2 [array[]] | main.rs:70:14:70:17 | arr2 | provenance |  |
+| main.rs:69:19:69:28 | source(...) | main.rs:69:9:69:12 | [post] arr2 [array[]] | provenance |  |
 nodes
 | main.rs:12:13:12:22 | source(...) | semmle.label | source(...) |
 | main.rs:13:10:13:14 | ... + ... | semmle.label | ... + ... |
 | main.rs:17:13:17:22 | source(...) | semmle.label | source(...) |
 | main.rs:18:10:18:11 | - ... | semmle.label | - ... |
 | main.rs:22:13:22:22 | source(...) | semmle.label | source(...) |
 | main.rs:24:10:24:17 | b as i64 | semmle.label | b as i64 |
+| main.rs:53:19:53:28 | source(...) | semmle.label | source(...) |
+| main.rs:54:14:54:19 | arr[1] | semmle.label | arr[1] |
+| main.rs:69:9:69:12 | [post] arr2 [array[]] | semmle.label | [post] arr2 [array[]] |
+| main.rs:69:19:69:28 | source(...) | semmle.label | source(...) |
+| main.rs:70:14:70:17 | arr2 | semmle.label | arr2 |
 subpaths
 testFailures
 #select
 | main.rs:13:10:13:14 | ... + ... | main.rs:12:13:12:22 | source(...) | main.rs:13:10:13:14 | ... + ... | $@ | main.rs:12:13:12:22 | source(...) | source(...) |
 | main.rs:18:10:18:11 | - ... | main.rs:17:13:17:22 | source(...) | main.rs:18:10:18:11 | - ... | $@ | main.rs:17:13:17:22 | source(...) | source(...) |
 | main.rs:24:10:24:17 | b as i64 | main.rs:22:13:22:22 | source(...) | main.rs:24:10:24:17 | b as i64 | $@ | main.rs:22:13:22:22 | source(...) | source(...) |
+| main.rs:54:14:54:19 | arr[1] | main.rs:53:19:53:28 | source(...) | main.rs:54:14:54:19 | arr[1] | $@ | main.rs:53:19:53:28 | source(...) | source(...) |
+| main.rs:70:14:70:17 | arr2 | main.rs:69:19:69:28 | source(...) | main.rs:70:14:70:17 | arr2 | $@ | main.rs:69:19:69:28 | source(...) | source(...) |

rust/ql/test/library-tests/dataflow/taint/main.rs

@@ -51,7 +51,7 @@ mod array_source {
 
     pub fn array_tainted() {
         let arr = source(76);
-        sink(arr[1]); // $ MISSING: hasTaintFlow=76
+        sink(arr[1]); // $ hasTaintFlow=76
     }
 }
 
@@ -67,7 +67,7 @@ mod array_sink {
     pub fn array_with_taint() {
         let mut arr2 = [1, 2, 3];
         arr2[1] = source(36);
-        sink(arr2); // $ MISSING: hasTaintFlow=36
+        sink(arr2); // $ hasTaintFlow=36
     }
 }