Merge pull request github#16552 from aschackmull/java/no-source-dispatch-for-exact-mad

Java: Remove source dispatch when there's an exact match from a manual model.

Author: aschackmull

java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

@@ -413,25 +413,28 @@ private string paramsStringQualified(Callable c) {
 }
 
 private Element interpretElement0(
-  string package, string type, boolean subtypes, string name, string signature
+  string package, string type, boolean subtypes, string name, string signature, boolean isExact
 ) {
   elementSpec(package, type, subtypes, name, signature, _) and
   (
     exists(Member m |
       (
-        result = m
+        result = m and isExact = true
         or
-        subtypes = true and result.(SrcMethod).overridesOrInstantiates+(m)
+        subtypes = true and result.(SrcMethod).overridesOrInstantiates+(m) and isExact = false
       ) and
       m.hasQualifiedName(package, type, name)
     |
-      signature = "" or
-      paramsStringQualified(m) = signature or
+      signature = ""
+      or
+      paramsStringQualified(m) = signature
+      or
       paramsString(m) = signature
     )
     or
     exists(RefType t |
       t.hasQualifiedName(package, type) and
+      isExact = false and
       (if subtypes = true then result.(SrcRefType).getASourceSupertype*() = t else result = t) and
       name = "" and
       signature = ""
@@ -442,13 +445,16 @@ private Element interpretElement0(
 /** Gets the source/sink/summary/neutral element corresponding to the supplied parameters. */
 cached
 Element interpretElement(
-  string package, string type, boolean subtypes, string name, string signature, string ext
+  string package, string type, boolean subtypes, string name, string signature, string ext,
+  boolean isExact
 ) {
   elementSpec(package, type, subtypes, name, signature, ext) and
-  exists(Element e | e = interpretElement0(package, type, subtypes, name, signature) |
-    ext = "" and result = e
+  exists(Element e, boolean isExact0 |
+    e = interpretElement0(package, type, subtypes, name, signature, isExact0)
+  |
+    ext = "" and result = e and isExact = isExact0
     or
-    ext = "Annotated" and result.(Annotatable).getAnAnnotation().getType() = e
+    ext = "Annotated" and result.(Annotatable).getAnAnnotation().getType() = e and isExact = false
   )
 }
 
@@ -538,13 +544,13 @@ predicate sinkNode(Node node, string kind) { sinkNode(node, kind, _) }
 
 // adapter class for converting Mad summaries to `SummarizedCallable`s
 private class SummarizedCallableAdapter extends SummarizedCallable {
-  SummarizedCallableAdapter() { summaryElement(this, _, _, _, _, _) }
+  SummarizedCallableAdapter() { summaryElement(this, _, _, _, _, _, _) }
 
   private predicate relevantSummaryElementManual(
     string input, string output, string kind, string model
   ) {
     exists(Provenance provenance |
-      summaryElement(this, input, output, kind, provenance, model) and
+      summaryElement(this, input, output, kind, provenance, model, _) and
       provenance.isManual()
     )
   }
@@ -553,11 +559,11 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
     string input, string output, string kind, string model
   ) {
     exists(Provenance provenance |
-      summaryElement(this, input, output, kind, provenance, model) and
+      summaryElement(this, input, output, kind, provenance, model, _) and
       provenance.isGenerated()
     ) and
     not exists(Provenance provenance |
-      neutralElement(this, "summary", provenance) and
+      neutralElement(this, "summary", provenance, _) and
       provenance.isManual()
     )
   }
@@ -576,18 +582,23 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
   }
 
   override predicate hasProvenance(Provenance provenance) {
-    summaryElement(this, _, _, _, provenance, _)
+    summaryElement(this, _, _, _, provenance, _, _)
   }
+
+  override predicate hasExactModel() { summaryElement(this, _, _, _, _, _, true) }
 }
 
 // adapter class for converting Mad neutrals to `NeutralCallable`s
 private class NeutralCallableAdapter extends NeutralCallable {
   string kind;
   string provenance_;
+  boolean exact;
 
-  NeutralCallableAdapter() { neutralElement(this, kind, provenance_) }
+  NeutralCallableAdapter() { neutralElement(this, kind, provenance_, exact) }
 
   override string getKind() { result = kind }
 
   override predicate hasProvenance(Provenance provenance) { provenance = provenance_ }
+
+  override predicate hasExactModel() { exact = true }
 }

java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll

@@ -135,6 +135,8 @@ private class SummarizedSyntheticCallableAdapter extends SummarizedCallable, TSy
       model = sc
     )
   }
+
+  override predicate hasExactModel() { any() }
 }
 
 deprecated class RequiredSummaryComponentStack = Impl::Private::RequiredSummaryComponentStack;

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll

@@ -19,7 +19,21 @@ private module DispatchImpl {
     )
   }
 
+  private predicate hasExactManualModel(Call c, Callable tgt) {
+    tgt = c.getCallee().getSourceDeclaration() and
+    (
+      exists(Impl::Public::SummarizedCallable sc |
+        sc.getACall() = c and sc.hasExactModel() and sc.hasManualModel()
+      )
+      or
+      exists(Impl::Public::NeutralSummaryCallable nc |
+        nc.getACall() = c and nc.hasExactModel() and nc.hasManualModel()
+      )
+    )
+  }
+
   private Callable sourceDispatch(Call c) {
+    not hasExactManualModel(c, result) and
     result = VirtualDispatch::viableCallable(c) and
     if VirtualDispatch::lowConfidenceDispatchTarget(c, result)
     then not hasHighConfidenceTarget(c)

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

@@ -131,7 +131,7 @@ private predicate relatedArgSpec(Callable c, string spec) {
     sourceModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _) or
     sinkModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _)
   |
-    c = interpretElement(namespace, type, subtypes, name, signature, ext)
+    c = interpretElement(namespace, type, subtypes, name, signature, ext, _)
   )
 }
 
@@ -202,7 +202,7 @@ module SourceSinkInterpretationInput implements
       sourceModel(namespace, type, subtypes, name, signature, ext, originalOutput, kind, provenance,
         madId) and
       model = "MaD:" + madId.toString() and
-      baseSource = interpretElement(namespace, type, subtypes, name, signature, ext) and
+      baseSource = interpretElement(namespace, type, subtypes, name, signature, ext, _) and
       (
         e = baseSource and output = originalOutput
         or
@@ -221,7 +221,7 @@ module SourceSinkInterpretationInput implements
       sinkModel(namespace, type, subtypes, name, signature, ext, originalInput, kind, provenance,
         madId) and
       model = "MaD:" + madId.toString() and
-      baseSink = interpretElement(namespace, type, subtypes, name, signature, ext) and
+      baseSink = interpretElement(namespace, type, subtypes, name, signature, ext, _) and
       (
         e = baseSink and originalInput = input
         or
@@ -310,7 +310,7 @@ module Private {
      */
     predicate summaryElement(
       Input::SummarizedCallableBase c, string input, string output, string kind, string provenance,
-      string model
+      string model, boolean isExact
     ) {
       exists(
         string namespace, string type, boolean subtypes, string name, string signature, string ext,
@@ -320,7 +320,7 @@ module Private {
         summaryModel(namespace, type, subtypes, name, signature, ext, originalInput, originalOutput,
           kind, provenance, madId) and
         model = "MaD:" + madId.toString() and
-        baseCallable = interpretElement(namespace, type, subtypes, name, signature, ext) and
+        baseCallable = interpretElement(namespace, type, subtypes, name, signature, ext, isExact) and
         (
           c.asCallable() = baseCallable and input = originalInput and output = originalOutput
           or
@@ -336,10 +336,12 @@ module Private {
      * Holds if a neutral model exists for `c` of kind `kind`
      * and with provenance `provenance`.
      */
-    predicate neutralElement(Input::SummarizedCallableBase c, string kind, string provenance) {
+    predicate neutralElement(
+      Input::SummarizedCallableBase c, string kind, string provenance, boolean isExact
+    ) {
       exists(string namespace, string type, string name, string signature |
         neutralModel(namespace, type, name, signature, kind, provenance) and
-        c.asCallable() = interpretElement(namespace, type, false, name, signature, "")
+        c.asCallable() = interpretElement(namespace, type, false, name, signature, "", isExact)
       )
     }
   }

java/ql/src/utils/modeleditor/ModelEditor.qll

@@ -77,7 +77,7 @@ class Endpoint extends Callable {
   predicate isNeutral() {
     exists(string namespace, string type, string name, string signature |
       neutralModel(namespace, type, name, signature, _, _) and
-      this = interpretElement(namespace, type, false, name, signature, "")
+      this = interpretElement(namespace, type, false, name, signature, "", _)
     )
   }
 

shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll

@@ -253,6 +253,13 @@ module Make<
        * that has provenance `provenance`.
        */
       predicate hasProvenance(Provenance provenance) { provenance = "manual" }
+
+      /**
+       * Holds if there exists a model for which this callable is an exact
+       * match, that is, no overriding was used to identify this callable from
+       * the model.
+       */
+      predicate hasExactModel() { none() }
     }
 
     final private class NeutralCallableFinal = NeutralCallable;
@@ -292,6 +299,13 @@ module Make<
        * Gets the kind of the neutral.
        */
       abstract string getKind();
+
+      /**
+       * Holds if there exists a model for which this callable is an exact
+       * match, that is, no overriding was used to identify this callable from
+       * the model.
+       */
+      predicate hasExactModel() { none() }
     }
   }