Merge pull request #17007 from michaelnebel/shared/neutralimplementation

C#/Java/Go: Neutrals are split into separate classes.

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

@@ -953,21 +953,3 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
     interpretSummary(this, _, _, _, provenance, _)
   }
 }
-
-// adapter class for converting Mad neutrals to `NeutralCallable`s
-private class NeutralCallableAdapter extends NeutralCallable {
-  string kind;
-  string provenance_;
-
-  NeutralCallableAdapter() {
-    // Neutral models have not been implemented for CPP.
-    none() and
-    exists(this) and
-    exists(kind) and
-    exists(provenance_)
-  }
-
-  override string getKind() { result = kind }
-
-  override predicate hasProvenance(Provenance provenance) { provenance = provenance_ }
-}

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

@@ -556,7 +556,7 @@ private predicate interpretSummary(
   )
 }
 
-private predicate interpretNeutral(UnboundCallable c, string kind, string provenance) {
+predicate interpretNeutral(UnboundCallable c, string kind, string provenance) {
   exists(string namespace, string type, string name, string signature |
     neutralModel(namespace, type, name, signature, kind, provenance) and
     c = interpretElement(namespace, type, false, name, signature, "")
@@ -613,18 +613,6 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
   }
 }
 
-// adapter class for converting Mad neutrals to `NeutralCallable`s
-private class NeutralCallableAdapter extends NeutralCallable {
-  string kind;
-  string provenance_;
-
-  NeutralCallableAdapter() { interpretNeutral(this, kind, provenance_) }
-
-  override string getKind() { result = kind }
-
-  override predicate hasProvenance(Provenance provenance) { provenance = provenance_ }
-}
-
 /**
  * A callable where there exists a MaD sink model that applies to it.
  */

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

@@ -16,6 +16,12 @@ private import semmle.code.csharp.dataflow.internal.ExternalFlow
 module Input implements InputSig<Location, DataFlowImplSpecific::CsharpDataFlow> {
   class SummarizedCallableBase = UnboundCallable;
 
+  predicate neutralElement(SummarizedCallableBase c, string kind, string provenance, boolean isExact) {
+    interpretNeutral(c, kind, provenance) and
+    // isExact is not needed for C#.
+    isExact = false
+  }
+
   ArgumentPosition callbackSelfParameterPosition() { result.isDelegateSelf() }
 
   ReturnKind getStandardReturnValueKind() { result instanceof NormalReturnKind }

csharp/ql/test/library-tests/dataflow/library/FlowSummaries.ql

@@ -1,10 +1,10 @@
 import shared.FlowSummaries
 import semmle.code.csharp.dataflow.internal.ExternalFlow
 
-final private class NeutralCallableFinal = NeutralCallable;
-
-class RelevantNeutralCallable extends NeutralCallableFinal {
-  final string getCallableCsv() { result = getSignature(this) }
+module R implements RelevantNeutralCallableSig<NeutralSummaryCallable> {
+  class RelevantNeutralCallable extends NeutralSummaryCallable {
+    final string getCallableCsv() { result = getSignature(this) }
+  }
 }
 
 class RelevantSourceCallable extends SourceCallable {
@@ -16,5 +16,5 @@ class RelevantSinkCallable extends SinkCallable {
 }
 
 import TestSummaryOutput<IncludeSummarizedCallable>
-import TestNeutralOutput<RelevantNeutralCallable>
+import TestNeutralOutput<NeutralSummaryCallable, R>
 import External::TestSourceSinkOutput<RelevantSourceCallable, RelevantSinkCallable>

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

@@ -4,3 +4,5 @@ extensions:
       extensible: neutralModel
     data:
       - [ "Models", "ManuallyModelled", "HasNeutralSummaryNoFlow", "(System.Object)", "summary", "manual"]
+      - [ "Sinks", "NewSinks", "NoSink", "(System.Object)", "summary", "df-generated"]
+      - [ "Sinks", "NewSinks", "NoSink", "(System.Object)", "sink", "manual"]

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

@@ -20,6 +20,10 @@ public class NewSinks
     // neutral=Sinks;NewSinks;Sink2;(System.Object);summary;df-generated
     public static void Sink2(object o) => throw null;
 
+    // Defined as sink neutral in the file next to the neutral summary test.
+    // neutral=Sinks;NewSinks;NoSink;(System.Object);summary;df-generated
+    public static void NoSink(object o) => throw 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

go/ql/lib/semmle/go/dataflow/ExternalFlow.qll

@@ -597,15 +597,3 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
     summaryElement(this, _, _, _, provenance, _)
   }
 }
-
-// adapter class for converting Mad neutrals to `NeutralCallable`s
-private class NeutralCallableAdapter extends NeutralCallable {
-  string kind;
-  string provenance_;
-
-  NeutralCallableAdapter() { neutralElement(this, kind, provenance_) }
-
-  override string getKind() { result = kind }
-
-  override predicate hasProvenance(Provenance provenance) { provenance = provenance_ }
-}

go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll

@@ -23,6 +23,17 @@ private string positionToString(int pos) {
 module Input implements InputSig<Location, DataFlowImplSpecific::GoDataFlow> {
   class SummarizedCallableBase = Callable;
 
+  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.asFunction() = interpretElement(namespace, type, false, name, signature, "").asEntity()
+    ) and
+    // isExact is not needed for Go.
+    isExact = false
+  }
+
   ArgumentPosition callbackSelfParameterPosition() { result = -1 }
 
   ReturnKind getStandardReturnValueKind() { result = getReturnKind(0) }
@@ -304,10 +315,7 @@ module Private {
      * and with provenance `provenance`.
      */
     predicate neutralElement(Input::SummarizedCallableBase c, string kind, string provenance) {
-      exists(string namespace, string type, string name, string signature |
-        neutralModel(namespace, type, name, signature, kind, provenance) and
-        c.asFunction() = interpretElement(namespace, type, false, name, signature, "").asEntity()
-      )
+      Input::neutralElement(c, kind, provenance, _)
     }
   }
 

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

@@ -636,21 +636,6 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
   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_, exact) }
-
-  override string getKind() { result = kind }
-
-  override predicate hasProvenance(Provenance provenance) { provenance = provenance_ }
-
-  override predicate hasExactModel() { exact = true }
-}
-
 /**
  * A callable where there exists a MaD sink model that applies to it.
  */

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

@@ -29,6 +29,15 @@ private string positionToString(int pos) {
 module Input implements InputSig<Location, DataFlowImplSpecific::JavaDataFlow> {
   class SummarizedCallableBase = FlowSummary::SummarizedCallableBase;
 
+  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, "", isExact)
+    )
+  }
+
   ArgumentPosition callbackSelfParameterPosition() { result = -1 }
 
   ReturnKind getStandardReturnValueKind() { any() }
@@ -332,18 +341,7 @@ 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, 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, "", isExact)
-      )
-    }
+    predicate neutralElement = Input::neutralElement/4;
   }
 
   /** Provides predicates for constructing summary components. */

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

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: neutralModel
+    data:
+      - [ "p", "Sinks", "nosink", "(Object)", "sink", "manual"]
+      - [ "p", "Sinks", "nosink", "(Object)", "summary", "df-generated"]

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

@@ -21,6 +21,10 @@ public class Sinks {
   // neutral=p;Sinks;sink2;(Object);summary;df-generated
   public void sink2(Object o) {}
 
+  // Defined as sink neutral in the file next to the neutral summary test.
+  // neutral=p;Sinks;nosink;(Object);summary;df-generated
+  public void nosink(Object o) {}
+
   // 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

@@ -19,6 +19,17 @@ signature module InputSig<LocationSig Location, DF::InputSig<Location> Lang> {
     string toString();
   }
 
+  /**
+   * Holds if a neutral (MaD) model exists for `c` of kind `kind`
+   * with provenance `provenance` and `isExact` is true if the model
+   * signature matches `c` exactly - otherwise false.
+   */
+  default predicate neutralElement(
+    SummarizedCallableBase c, string kind, string provenance, boolean isExact
+  ) {
+    none()
+  }
+
   /** Gets the parameter position representing a callback itself, if any. */
   default Lang::ArgumentPosition callbackSelfParameterPosition() { none() }
 
@@ -261,64 +272,106 @@ module Make<
       predicate hasExactModel() { none() }
     }
 
-    final private class NeutralCallableFinal = NeutralCallable;
-
-    /**
-     * A callable where there is no flow via the callable.
-     */
-    class NeutralSummaryCallable extends NeutralCallableFinal {
-      NeutralSummaryCallable() { this.getKind() = "summary" }
-    }
-
-    /**
-     * A callable that has a neutral source model.
-     */
-    class NeutralSourceCallable extends NeutralCallableFinal {
-      NeutralSourceCallable() { this.getKind() = "source" }
-    }
-
-    /**
-     * A callable that has a neutral sink model.
-     */
-    class NeutralSinkCallable extends NeutralCallableFinal {
-      NeutralSinkCallable() { this.getKind() = "sink" }
-    }
-
-    /**
-     * A callable that has a neutral model.
-     */
-    abstract class NeutralCallable extends SummarizedCallableBaseFinal {
-      bindingset[this]
-      NeutralCallable() { exists(this) }
-
-      /**
-       * Holds if the neutral is auto generated.
-       */
-      final predicate hasGeneratedModel() {
-        any(Provenance p | this.hasProvenance(p)).isGenerated()
-      }
-
-      /**
-       * Holds if there exists a manual neutral that applies to this callable.
-       */
-      final predicate hasManualModel() { any(Provenance p | this.hasProvenance(p)).isManual() }
+    private signature predicate hasKindSig(string kind);
 
+    signature class NeutralCallableSig extends SummarizedCallableBaseFinal {
       /**
        * Holds if the neutral has provenance `p`.
        */
-      abstract predicate hasProvenance(Provenance p);
+      predicate hasProvenance(Provenance p);
 
       /**
        * Gets the kind of the neutral.
        */
-      abstract string getKind();
+      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.
+       * Holds if the neutral is auto generated.
        */
-      predicate hasExactModel() { none() }
+      predicate hasGeneratedModel();
+
+      /**
+       * Holds if there exists a manual neutral that applies to this callable.
+       */
+      predicate hasManualModel();
+    }
+
+    /**
+     * A module for constructing classes of neutral callables.
+     */
+    private module MakeNeutralCallable<hasKindSig/1 hasKind> {
+      class NeutralCallable extends SummarizedCallableBaseFinal {
+        private string kind;
+        private string provenance_;
+        private boolean exact;
+
+        NeutralCallable() {
+          hasKind(kind) and
+          neutralElement(this, kind, provenance_, exact)
+        }
+
+        /**
+         * Gets the kind of the neutral.
+         */
+        string getKind() { result = kind }
+
+        /**
+         * Holds if the neutral has provenance `p`.
+         */
+        predicate hasProvenance(Provenance provenance) { provenance = provenance_ }
+
+        /**
+         * Holds if the neutral is auto generated.
+         */
+        final predicate hasGeneratedModel() {
+          any(Provenance p | this.hasProvenance(p)).isGenerated()
+        }
+
+        /**
+         * Holds if there exists a manual neutral that applies to this callable.
+         */
+        final predicate hasManualModel() { any(Provenance p | this.hasProvenance(p)).isManual() }
+
+        /**
+         * 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() { exact = true }
+      }
+    }
+
+    private predicate neutralSummaryKind(string kind) { kind = "summary" }
+
+    /**
+     * A callable where there exists a MaD neutral summary model that applies to it.
+     */
+    class NeutralSummaryCallable = MakeNeutralCallable<neutralSummaryKind/1>::NeutralCallable;
+
+    private predicate neutralSourceKind(string kind) { kind = "source" }
+
+    /**
+     * A callable where there exists a MaD neutral source model that applies to it.
+     */
+    class NeutralSourceCallable = MakeNeutralCallable<neutralSourceKind/1>::NeutralCallable;
+
+    private predicate neutralSinkKind(string kind) { kind = "sink" }
+
+    /**
+     * A callable where there exists a MaD neutral sink model that applies to it.
+     */
+    class NeutralSinkCallable = MakeNeutralCallable<neutralSinkKind/1>::NeutralCallable;
+
+    /**
+     * A callable where there exist a MaD neutral (summary, source or sink) model
+     * that applies to it.
+     */
+    class NeutralCallable extends SummarizedCallableBaseFinal {
+      NeutralCallable() {
+        this instanceof NeutralSummaryCallable or
+        this instanceof NeutralSourceCallable or
+        this instanceof NeutralSinkCallable
+      }
     }
   }
 
@@ -1880,13 +1933,20 @@ module Make<
     }
 
     /** A summarized callable relevant for testing. */
-    signature class RelevantNeutralCallableSig extends NeutralCallable {
-      /** Gets the string representation of this callable used by `neutral/1`. */
-      string getCallableCsv();
+    signature module RelevantNeutralCallableSig<NeutralCallableSig NeutralCallableInput> {
+      class RelevantNeutralCallable extends NeutralCallableInput {
+        /** Gets the string representation of this callable used by `neutral/1`. */
+        string getCallableCsv();
+      }
     }
 
-    module TestNeutralOutput<RelevantNeutralCallableSig RelevantNeutralCallable> {
-      private string renderProvenance(NeutralCallable c) {
+    module TestNeutralOutput<
+      NeutralCallableSig NeutralCallableInput,
+      RelevantNeutralCallableSig<NeutralCallableInput> RelevantNeutralCallableInput>
+    {
+      class RelevantNeutralCallable = RelevantNeutralCallableInput::RelevantNeutralCallable;
+
+      private string renderProvenance(NeutralCallableInput c) {
         exists(Provenance p | p.isManual() and c.hasProvenance(p) and result = p.toString())
         or
         not c.hasManualModel() and

swift/ql/lib/codeql/swift/dataflow/ExternalFlow.qll

@@ -574,21 +574,3 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
     interpretSummary(this, _, _, _, provenance, _)
   }
 }
-
-// adapter class for converting Mad neutrals to `NeutralCallable`s
-private class NeutralCallableAdapter extends NeutralCallable {
-  string kind;
-  string provenance_;
-
-  NeutralCallableAdapter() {
-    // Neutral models have not been implemented for Swift.
-    none() and
-    exists(this) and
-    exists(kind) and
-    exists(provenance_)
-  }
-
-  override string getKind() { result = kind }
-
-  override predicate hasProvenance(Provenance provenance) { provenance = provenance_ }
-}