Merge pull request #17219 from michaelnebel/shared/neutralsourcesink

C#/Java: Fix source- and sink callable provenance overlap.

Author: michaelnebel

csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll

@@ -613,32 +613,6 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
   }
 }
 
-/**
- * A callable where there exists a MaD sink model that applies to it.
- */
-private class SinkModelCallableAdapter extends SinkModelCallable {
-  private Provenance provenance;
-
-  SinkModelCallableAdapter() {
-    SourceSinkInterpretationInput::sinkElement(this, _, _, provenance, _)
-  }
-
-  override predicate hasProvenance(Provenance p) { provenance = p }
-}
+final class SourceCallable = SourceModelCallable;
 
 final class SinkCallable = SinkModelCallable;
-
-/**
- * A callable where there exists a MaD source model that applies to it.
- */
-private class SourceModelCallableAdapter extends SourceModelCallable {
-  private Provenance provenance;
-
-  SourceModelCallableAdapter() {
-    SourceSinkInterpretationInput::sourceElement(this, _, _, provenance, _)
-  }
-
-  override predicate hasProvenance(Provenance p) { provenance = p }
-}
-
-final class SourceCallable = SourceModelCallable;

csharp/ql/test/utils/modelgenerator/dataflow/CaptureSinkModels.ext.yml

@@ -6,6 +6,13 @@ extensions:
       - [ "Sinks", "NewSinks", False, "Sink", "(System.Object)", "", "Argument[0]", "test-sink", "manual"]
       - [ "Sinks", "NewSinks", False, "Sink2", "(System.Object)", "", "Argument[0]", "test-sink2", "manual"]
       - [ "Sinks", "NewSinks", False, "ManualSinkAlreadyDefined", "(System.Object)", "", "Argument[0]", "test-sink", "manual"]
+      - [ "Sinks", "NewSinks", False, "SaveAndGet", "(System.Object)", "", "Argument[0]", "test-sink", "df-generated"]
+
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: sourceModel
+    data:
+      - [ "Sinks", "NewSinks", False, "SaveAndGet", "(System.Object)", "", "ReturnValue", "test-source", "manual"]
 
   - addsTo:
       pack: codeql/csharp-all

csharp/ql/test/utils/modelgenerator/dataflow/Sinks.cs

@@ -24,6 +24,15 @@ public class NewSinks
     // neutral=Sinks;NewSinks;NoSink;(System.Object);summary;df-generated
     public static void NoSink(object o) => throw null;
 
+    // Sink and Source defined in the extensible file next to the sink test.
+    // sink=Sinks;NewSinks;false;SaveAndGet;(System.Object);;Argument[0];test-sink;df-generated
+    // neutral=Sinks;NewSinks;SaveAndGet;(System.Object);summary;df-generated
+    public static object SaveAndGet(object o)
+    {
+        Sink(o);
+        return null;
+    }
+
     // New sink
     // sink=Sinks;NewSinks;false;WrapResponseWrite;(System.Object);;Argument[0];html-injection;df-generated
     // neutral=Sinks;NewSinks;WrapResponseWrite;(System.Object);summary;df-generated

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

@@ -636,32 +636,6 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
   override predicate hasExactModel() { summaryElement(this, _, _, _, _, _, true) }
 }
 
-/**
- * A callable where there exists a MaD sink model that applies to it.
- */
-private class SinkModelCallableAdapter extends SinkModelCallable {
-  private Provenance provenance;
-
-  SinkModelCallableAdapter() {
-    SourceSinkInterpretationInput::sinkElement(this, _, _, provenance, _)
-  }
-
-  override predicate hasProvenance(Provenance p) { provenance = p }
-}
-
 final class SinkCallable = SinkModelCallable;
 
-/**
- * A callable where there exists a MaD source model that applies to it.
- */
-private class SourceModelCallableAdapter extends SourceModelCallable {
-  private Provenance provenance;
-
-  SourceModelCallableAdapter() {
-    SourceSinkInterpretationInput::sourceElement(this, _, _, provenance, _)
-  }
-
-  override predicate hasProvenance(Provenance p) { provenance = p }
-}
-
 final class SourceCallable = SourceModelCallable;

java/ql/test/utils/modelgenerator/dataflow/CaptureSinkModels.ext.yml

@@ -7,6 +7,13 @@ extensions:
       - [ "p", "Sinks", False, "sink", "(Object)", "", "Argument[0]", "test-sink", "manual" ]
       - [ "p", "Sinks", False, "sink2", "(Object)", "", "Argument[0]", "test-sink2", "manual" ]
       - [ "p", "Sinks", False, "manualSinkAlreadyDefined", "(Object)", "", "Argument[0]", "test-sink", "manual" ]
+      - [ "p", "Sinks", False, "saveAndGet", "(Object)", "", "Argument[0]", "test-sink", "df-generated"]
+
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sourceModel
+    data:
+      - [ "p", "Sinks", False, "saveAndGet", "(Object)", "", "ReturnValue", "test-source", "manual"]
 
   - addsTo:
       pack: codeql/java-all

java/ql/test/utils/modelgenerator/dataflow/p/Sinks.java

@@ -25,6 +25,14 @@ public void sink2(Object o) {}
   // neutral=p;Sinks;nosink;(Object);summary;df-generated
   public void nosink(Object o) {}
 
+  // Sink and Source defined in the extensible file next to the sink test.
+  // sink=p;Sinks;true;saveAndGet;(Object);;Argument[0];test-sink;df-generated
+  // neutral=p;Sinks;saveAndGet;(Object);summary;df-generated
+  public Object saveAndGet(Object o) {
+    sink(o);
+    return null;
+  }
+
   // sink=p;Sinks;true;copyFileToDirectory;(Path,Path,CopyOption[]);;Argument[0];path-injection;df-generated
   // sink=p;Sinks;true;copyFileToDirectory;(Path,Path,CopyOption[]);;Argument[1];path-injection;df-generated
   // neutral=p;Sinks;copyFileToDirectory;(Path,Path,CopyOption[]);summary;df-generated

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

@@ -1808,34 +1808,37 @@ module Make<
 
         final private class SourceOrSinkElementFinal = SourceOrSinkElement;
 
-        bindingset[this]
-        abstract private class SourceSinkModelCallableBase extends SourceOrSinkElementFinal {
-          /**
-           * Holds if there exists a manual model that applies to this.
-           */
-          final predicate hasManualModel() { any(Provenance p | this.hasProvenance(p)).isManual() }
+        signature predicate sourceOrSinkElementSig(
+          Element e, string path, string kind, Provenance provenance, string model
+        );
 
-          /**
-           * Holds if this has provenance `p`.
-           */
-          abstract predicate hasProvenance(Provenance p);
+        private module MakeSourceOrSinkCallable<sourceOrSinkElementSig/5 sourceOrSinkElement> {
+          class SourceSinkCallable extends SourceOrSinkElementFinal {
+            private Provenance provenance;
+
+            SourceSinkCallable() { sourceOrSinkElement(this, _, _, provenance, _) }
+
+            /**
+             * Holds if there exists a manual model that applies to this.
+             */
+            predicate hasManualModel() { any(Provenance p | this.hasProvenance(p)).isManual() }
+
+            /**
+             * Holds if this has provenance `p`.
+             */
+            predicate hasProvenance(Provenance p) { provenance = p }
+          }
         }
 
         /**
          * A callable that has a source model.
          */
-        abstract class SourceModelCallable extends SourceSinkModelCallableBase {
-          bindingset[this]
-          SourceModelCallable() { exists(this) }
-        }
+        class SourceModelCallable = MakeSourceOrSinkCallable<sourceElement/5>::SourceSinkCallable;
 
         /**
          * A callable that has a sink model.
          */
-        abstract class SinkModelCallable extends SourceSinkModelCallableBase {
-          bindingset[this]
-          SinkModelCallable() { exists(this) }
-        }
+        class SinkModelCallable = MakeSourceOrSinkCallable<sinkElement/5>::SourceSinkCallable;
 
         /** A source or sink relevant for testing. */
         signature class RelevantSourceOrSinkElementSig extends SourceOrSinkElement {