Rust: Source/sink/barrier MaD trait models apply to implementations

Author: hvitved

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

@@ -174,6 +174,24 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
   )
 }
 
+bindingset[path, orig]
+pragma[inline_late]
+private predicate interpretPath(
+  string path, Function f, Provenance orig, Provenance p, boolean isExact
+) {
+  exists(Function f0 | f0.getCanonicalPath() = path |
+    f = f0 and
+    isExact = true and
+    p = orig
+    or
+    f.implements(f0) and
+    isExact = false and
+    // making inherited models generated means that source code definitions and
+    // exact generated models take precedence
+    p = "hq-generated"
+  )
+}
+
 private class SummarizedCallableFromModel extends SummarizedCallable::Range {
   string input_;
   string output_;
@@ -183,19 +201,9 @@ private class SummarizedCallableFromModel extends SummarizedCallable::Range {
   QlBuiltins::ExtensionId madId;
 
   SummarizedCallableFromModel() {
-    exists(string path, Function f, Provenance p |
+    exists(string path, Provenance p |
       summaryModel(path, input_, output_, kind, p, madId) and
-      f.getCanonicalPath() = path
-    |
-      this = f and
-      isExact_ = true and
-      p_ = p
-      or
-      this.implements(f) and
-      isExact_ = false and
-      // making inherited models generated means that source code definitions and
-      // exact generated models take precedence
-      p_ = "hq-generated"
+      interpretPath(path, this, p, p_, isExact_)
     )
   }
 
@@ -246,16 +254,20 @@ private class SummarizedCallableWithCallback extends SummarizedCallable::Range {
 
 private class FlowSourceFromModel extends FlowSource::Range {
   private string path;
+  private Provenance p_;
 
   FlowSourceFromModel() {
-    sourceModel(path, _, _, _, _) and
-    this.getCanonicalPath() = path
+    exists(Provenance p |
+      sourceModel(path, _, _, p, _) and
+      interpretPath(path, this, p, p_, _)
+    )
   }
 
   override predicate isSource(string output, string kind, Provenance provenance, string model) {
     exists(QlBuiltins::ExtensionId madId |
-      sourceModel(path, output, kind, provenance, madId) and
-      model = "MaD:" + madId.toString()
+      sourceModel(path, output, kind, _, madId) and
+      model = "MaD:" + madId.toString() and
+      provenance = p_
     ) and
     // Only apply generated models when no neutral model exists
     // (the shared code only applies neutral models to summaries at present)
@@ -268,16 +280,20 @@ private class FlowSourceFromModel extends FlowSource::Range {
 
 private class FlowSinkFromModel extends FlowSink::Range {
   private string path;
+  private Provenance p_;
 
   FlowSinkFromModel() {
-    sinkModel(path, _, _, _, _) and
-    this.getCanonicalPath() = path
+    exists(Provenance p |
+      sinkModel(path, _, _, p, _) and
+      interpretPath(path, this, p, p_, _)
+    )
   }
 
   override predicate isSink(string input, string kind, Provenance provenance, string model) {
     exists(QlBuiltins::ExtensionId madId |
-      sinkModel(path, input, kind, provenance, madId) and
-      model = "MaD:" + madId.toString()
+      sinkModel(path, input, kind, _, madId) and
+      model = "MaD:" + madId.toString() and
+      provenance = p_
     ) and
     // Only apply generated models when no neutral model exists
     // (the shared code only applies neutral models to summaries at present)
@@ -290,34 +306,42 @@ private class FlowSinkFromModel extends FlowSink::Range {
 
 private class FlowBarrierFromModel extends FlowBarrier::Range {
   private string path;
+  private Provenance p_;
 
   FlowBarrierFromModel() {
-    barrierModel(path, _, _, _, _) and
-    this.getCanonicalPath() = path
+    exists(Provenance p |
+      barrierModel(path, _, _, p, _) and
+      interpretPath(path, this, p, p_, _)
+    )
   }
 
   override predicate isBarrier(string output, string kind, Provenance provenance, string model) {
     exists(QlBuiltins::ExtensionId madId |
-      barrierModel(path, output, kind, provenance, madId) and
-      model = "MaD:" + madId.toString()
+      barrierModel(path, output, kind, _, madId) and
+      model = "MaD:" + madId.toString() and
+      provenance = p_
     )
   }
 }
 
 private class FlowBarrierGuardFromModel extends FlowBarrierGuard::Range {
   private string path;
+  private Provenance p_;
 
   FlowBarrierGuardFromModel() {
-    barrierGuardModel(path, _, _, _, _, _) and
-    this.getCanonicalPath() = path
+    exists(Provenance p |
+      barrierGuardModel(path, _, _, _, p, _) and
+      interpretPath(path, this, p, p_, _)
+    )
   }
 
   override predicate isBarrierGuard(
     string input, string acceptingValue, string kind, Provenance provenance, string model
   ) {
     exists(QlBuiltins::ExtensionId madId |
-      barrierGuardModel(path, input, acceptingValue, kind, provenance, madId) and
-      model = "MaD:" + madId.toString()
+      barrierGuardModel(path, input, acceptingValue, kind, _, madId) and
+      model = "MaD:" + madId.toString() and
+      provenance = p_
     )
   }
 }

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

@@ -4,8 +4,6 @@ edges
 | main.rs:21:13:21:21 | source(...) | main.rs:21:9:21:9 | s | provenance |  |
 | main.rs:32:9:32:9 | s | main.rs:33:10:33:10 | s | provenance |  |
 | main.rs:32:13:32:21 | source(...) | main.rs:32:9:32:9 | s | provenance |  |
-| main.rs:63:9:63:9 | s | main.rs:65:10:65:10 | s | provenance |  |
-| main.rs:63:13:63:21 | source(...) | main.rs:63:9:63:9 | s | provenance |  |
 nodes
 | main.rs:17:10:17:18 | source(...) | semmle.label | source(...) |
 | main.rs:21:9:21:9 | s | semmle.label | s |
@@ -14,13 +12,9 @@ nodes
 | main.rs:32:9:32:9 | s | semmle.label | s |
 | main.rs:32:13:32:21 | source(...) | semmle.label | source(...) |
 | main.rs:33:10:33:10 | s | semmle.label | s |
-| main.rs:63:9:63:9 | s | semmle.label | s |
-| main.rs:63:13:63:21 | source(...) | semmle.label | source(...) |
-| main.rs:65:10:65:10 | s | semmle.label | s |
 subpaths
 testFailures
 #select
 | main.rs:17:10:17:18 | source(...) | main.rs:17:10:17:18 | source(...) | main.rs:17:10:17:18 | source(...) | $@ | main.rs:17:10:17:18 | source(...) | source(...) |
 | main.rs:22:10:22:10 | s | main.rs:21:13:21:21 | source(...) | main.rs:22:10:22:10 | s | $@ | main.rs:21:13:21:21 | source(...) | source(...) |
 | main.rs:33:10:33:10 | s | main.rs:32:13:32:21 | source(...) | main.rs:33:10:33:10 | s | $@ | main.rs:32:13:32:21 | source(...) | source(...) |
-| main.rs:65:10:65:10 | s | main.rs:63:13:63:21 | source(...) | main.rs:65:10:65:10 | s | $@ | main.rs:63:13:63:21 | source(...) | source(...) |

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

@@ -62,7 +62,7 @@ impl<T> MyBarrierTrait3 for T {
 fn with_trait_barriers() {
     let s = source(2);
     <()>::sanitize2(s);
-    sink(s); // $ SPURIOUS: hasValueFlow=2
+    sink(s);
     <()>::sanitize3(s);
     sink(s);
 }

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

@@ -527,12 +527,12 @@ fn test_trait_model<T: Ord>(x: T) {
     sink(x9); // $ hasValueFlow=30
 
     let x10 = <()>::produce2(31);
-    sink(x10); // $ MISSING: hasValueFlow=31
+    sink(x10); // $ hasValueFlow=31
 
     let x11 = <()>::produce3(32);
     sink(x11); // $ hasValueFlow=32
 
-    <()>::consume2(source(33)); // $ MISSING: hasValueFlow=33
+    <()>::consume2(source(33)); // $ hasValueFlow=33
 
     <()>::consume3(source(34)); // $ hasValueFlow=34
 }