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

Author: hvitved

cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll

@@ -17,10 +17,6 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
 
   class SummarizedCallableBase = Function;
 
-  class SourceBase = Function;
-
-  class SinkBase = Function;
-
   class FlowSummaryCallBase = CallInstruction;
 
   predicate callableFromSource(SummarizedCallableBase c) { exists(c.getBlock()) }
@@ -232,7 +228,7 @@ private module Input2 implements Impl::Private::InputSig2 {
 
   bindingset[source, sc]
   SourceSinkReportingElement getASourceReportingElement(
-    Input::SourceBase source, Impl::Private::SummaryComponent sc
+    Input::SummarizedCallableBase source, Impl::Private::SummaryComponent sc
   ) {
     exists(Call call | call.getTarget() = source |
       sc = Impl::Private::SummaryComponent::return(_) and
@@ -312,7 +308,7 @@ private module Input2 implements Impl::Private::InputSig2 {
 
   bindingset[sink, sc]
   SourceSinkReportingElement getASinkReportingElement(
-    Input::SinkBase sink, Impl::Private::SummaryComponent sc
+    Input::SummarizedCallableBase sink, Impl::Private::SummaryComponent sc
   ) {
     exists(Call call, ArgumentPosition pos |
       call.getTarget() = sink and
@@ -525,9 +521,10 @@ private class SourceModelFunction extends Public::SourceElement instanceof Funct
   }
 
   override predicate isSource(
-    string output, string kind, Public::Provenance provenance, string model
+    string output, string kind, Public::Provenance provenance, boolean isExact, string model
   ) {
-    sourceModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance, model)
+    sourceModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance, model) and
+    isExact = true
   }
 }
 
@@ -544,7 +541,10 @@ private class SinkModelFunction extends Public::SinkElement instanceof Function
     this = interpretElement(namespace, type, subtypes, name, signature, ext)
   }
 
-  override predicate isSink(string input, string kind, Public::Provenance provenance, string model) {
-    sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance, model)
+  override predicate isSink(
+    string input, string kind, Public::Provenance provenance, boolean isExact, string model
+  ) {
+    sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance, model) and
+    isExact = true
   }
 }

rust/ql/lib/codeql/rust/dataflow/FlowBarrier.qll

@@ -44,9 +44,9 @@ module FlowBarrier {
     Range() { any() }
 
     override predicate isBarrier(
-      string output, string kind, Impl::Public::Provenance provenance, string model
+      string output, string kind, Impl::Public::Provenance provenance, boolean isExact, string model
     ) {
-      this.isBarrier(output, kind) and provenance = "manual" and model = ""
+      this.isBarrier(output, kind) and provenance = "manual" and isExact = true and model = ""
     }
 
     /**
@@ -67,9 +67,13 @@ module FlowBarrierGuard {
     Range() { any() }
 
     override predicate isBarrierGuard(
-      string input, string branch, string kind, Impl::Public::Provenance provenance, string model
+      string input, string branch, string kind, Impl::Public::Provenance provenance,
+      boolean isExact, string model
     ) {
-      this.isBarrierGuard(input, branch, kind) and provenance = "manual" and model = ""
+      this.isBarrierGuard(input, branch, kind) and
+      provenance = "manual" and
+      isExact = true and
+      model = ""
     }
 
     /**

rust/ql/lib/codeql/rust/dataflow/FlowSink.qll

@@ -36,9 +36,9 @@ module FlowSink {
     Range() { any() }
 
     override predicate isSink(
-      string input, string kind, Impl::Public::Provenance provenance, string model
+      string input, string kind, Impl::Public::Provenance provenance, boolean isExact, string model
     ) {
-      this.isSink(input, kind) and provenance = "manual" and model = ""
+      this.isSink(input, kind) and provenance = "manual" and isExact = true and model = ""
     }
 
     /**

rust/ql/lib/codeql/rust/dataflow/FlowSource.qll

@@ -42,9 +42,9 @@ module FlowSource {
     Range() { any() }
 
     override predicate isSource(
-      string output, string kind, Impl::Public::Provenance provenance, string model
+      string output, string kind, Impl::Public::Provenance provenance, boolean isExact, string model
     ) {
-      this.isSource(output, kind) and provenance = "manual" and model = ""
+      this.isSource(output, kind) and provenance = "manual" and isExact = true and model = ""
     }
 
     /**

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

@@ -43,17 +43,12 @@ module Input implements InputSig<Location, RustDataFlow> {
     result.asSummarizedCallable() = c
   }
 
-  class SourceBase = Function;
-
-  class SinkBase = Function;
-
   predicate neutralElement(
     Input::SummarizedCallableBase c, string kind, string provenance, boolean isExact
   ) {
-    exists(string path |
-      neutralModel(path, kind, provenance, _) and
-      c.getCanonicalPath() = path and
-      isExact = true
+    exists(string path, Provenance orig |
+      neutralModel(path, kind, orig, _) and
+      interpretPath(path, c, orig, provenance, isExact)
     )
   }
 
@@ -170,7 +165,7 @@ module Input2 implements Impl::Private::InputSig2 {
   }
 
   SourceSinkReportingElement getASourceReportingElement(
-    Input::SourceBase source, Impl::Private::SummaryComponent sc
+    Input::SummarizedCallableBase source, Impl::Private::SummaryComponent sc
   ) {
     exists(Call call | call.getResolvedTarget() = source |
       sc = Impl::Private::SummaryComponent::return(_) and
@@ -200,7 +195,7 @@ module Input2 implements Impl::Private::InputSig2 {
   }
 
   SourceSinkReportingElement getASinkReportingElement(
-    Input::SinkBase sink, Impl::Private::SummaryComponent sc
+    Input::SummarizedCallableBase sink, Impl::Private::SummaryComponent sc
   ) {
     exists(Call call |
       call.getResolvedTarget() = sink and

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

@@ -174,6 +174,22 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
   )
 }
 
+bindingset[path, orig]
+pragma[inline_late]
+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 +199,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,78 +252,93 @@ private class SummarizedCallableWithCallback extends SummarizedCallable::Range {
 
 private class FlowSourceFromModel extends FlowSource::Range {
   private string path;
+  private Provenance orig;
+  private Provenance p_;
+  private boolean isExact_;
 
   FlowSourceFromModel() {
-    sourceModel(path, _, _, _, _) and
-    this.getCanonicalPath() = path
+    sourceModel(path, _, _, orig, _) and
+    interpretPath(path, this, orig, p_, isExact_)
   }
 
-  override predicate isSource(string output, string kind, Provenance provenance, string model) {
+  override predicate isSource(
+    string output, string kind, Provenance provenance, boolean isExact, string model
+  ) {
     exists(QlBuiltins::ExtensionId madId |
-      sourceModel(path, output, kind, provenance, madId) and
-      model = "MaD:" + madId.toString()
-    ) and
-    // Only apply generated models when no neutral model exists
-    // (the shared code only applies neutral models to summaries at present)
-    not (
-      provenance.isGenerated() and
-      neutralModel(path, "source", _, _)
+      sourceModel(path, output, kind, orig, madId) and
+      model = "MaD:" + madId.toString() and
+      provenance = p_ and
+      isExact = isExact_
     )
   }
 }
 
 private class FlowSinkFromModel extends FlowSink::Range {
   private string path;
+  private Provenance orig;
+  private Provenance p_;
+  private boolean isExact_;
 
   FlowSinkFromModel() {
-    sinkModel(path, _, _, _, _) and
-    this.getCanonicalPath() = path
+    sinkModel(path, _, _, orig, _) and
+    interpretPath(path, this, orig, p_, isExact_)
   }
 
-  override predicate isSink(string input, string kind, Provenance provenance, string model) {
+  override predicate isSink(
+    string input, string kind, Provenance provenance, boolean isExact, string model
+  ) {
     exists(QlBuiltins::ExtensionId madId |
-      sinkModel(path, input, kind, provenance, madId) and
-      model = "MaD:" + madId.toString()
-    ) and
-    // Only apply generated models when no neutral model exists
-    // (the shared code only applies neutral models to summaries at present)
-    not (
-      provenance.isGenerated() and
-      neutralModel(path, "sink", _, _)
+      sinkModel(path, input, kind, orig, madId) and
+      model = "MaD:" + madId.toString() and
+      provenance = p_ and
+      isExact = isExact_
     )
   }
 }
 
 private class FlowBarrierFromModel extends FlowBarrier::Range {
   private string path;
+  private Provenance orig;
+  private Provenance p_;
+  private boolean isExact_;
 
   FlowBarrierFromModel() {
-    barrierModel(path, _, _, _, _) and
-    this.getCanonicalPath() = path
+    barrierModel(path, _, _, orig, _) and
+    interpretPath(path, this, orig, p_, isExact_)
   }
 
-  override predicate isBarrier(string output, string kind, Provenance provenance, string model) {
+  override predicate isBarrier(
+    string output, string kind, Provenance provenance, boolean isExact, 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_ and
+      isExact = isExact_
     )
   }
 }
 
 private class FlowBarrierGuardFromModel extends FlowBarrierGuard::Range {
   private string path;
+  private Provenance orig;
+  private Provenance p_;
+  private boolean isExact_;
 
   FlowBarrierGuardFromModel() {
-    barrierGuardModel(path, _, _, _, _, _) and
-    this.getCanonicalPath() = path
+    barrierGuardModel(path, _, _, _, orig, _) and
+    interpretPath(path, this, orig, p_, isExact_)
   }
 
   override predicate isBarrierGuard(
-    string input, string acceptingValue, string kind, Provenance provenance, string model
+    string input, string acceptingValue, string kind, Provenance provenance, boolean isExact,
+    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_ and
+      isExact = isExact_
     )
   }
 }

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
 }