Merge pull request github#17521 from michaelnebel/modelgen/moreimprov…

…ements C#/Java: Content based model generation improvements.
jessejay-ch · Sep 30, 2024 · 6f74387 · 6f74387
2 parents 4513643 + baae8d0
commit 6f74387
Show file tree

Hide file tree

Showing 40 changed files with 626 additions and 251 deletions.
diff --git a/csharp/ql/src/utils/modelgenerator/CaptureContentSummaryModels.ql b/csharp/ql/src/utils/modelgenerator/CaptureContentSummaryModels.ql
@@ -9,5 +9,5 @@
 import internal.CaptureModels
 
 from DataFlowSummaryTargetApi api, string flow
-where flow = ContentSensitive::captureFlow(api)
+where flow = ContentSensitive::captureFlow(api, _)
 select flow order by flow
diff --git a/csharp/ql/src/utils/modelgenerator/CaptureMixedNeutralModels.ql b/csharp/ql/src/utils/modelgenerator/CaptureMixedNeutralModels.ql
@@ -0,0 +1,13 @@
+/**
+ * @name Capture mixed neutral models.
+ * @description Finds neutral models to be used by other queries.
+ * @kind diagnostic
+ * @id cs/utils/modelgenerator/mixed-neutral-models
+ * @tags modelgenerator
+ */
+
+import internal.CaptureModels
+
+from DataFlowSummaryTargetApi api, string noflow
+where noflow = captureMixedNeutral(api)
+select noflow order by noflow
diff --git a/csharp/ql/src/utils/modelgenerator/CaptureMixedSummaryModels.ql b/csharp/ql/src/utils/modelgenerator/CaptureMixedSummaryModels.ql
@@ -0,0 +1,13 @@
+/**
+ * @name Capture mixed summary models.
+ * @description Finds applicable summary models to be used by other queries.
+ * @kind diagnostic
+ * @id cs/utils/modelgenerator/mixed-summary-models
+ * @tags modelgenerator
+ */
+
+import internal.CaptureModels
+
+from DataFlowSummaryTargetApi api, string flow
+where flow = captureMixedFlow(api, _)
+select flow order by flow
diff --git a/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll
@@ -131,7 +131,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
 
     Callable lift() { result = lift }
 
-    predicate isRelevant() { relevant(this) }
+    predicate isRelevant() {
+      relevant(this) and
+      not hasManualSummaryModel(this)
+    }
   }
 
   /**

diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs
@@ -293,7 +293,8 @@ static void Sink(object o) { }
     }
 
     // Test synthetic fields
-    public class K {
+    public class K
+    {
 
         public object MyField;
 
@@ -309,24 +310,42 @@ public class K {
 
         public object GetMyFieldOnSyntheticField() => throw null;
 
-        public void M1() {
+        public void M1()
+        {
             var o = new object();
             SetMySyntheticField(o);
             Sink(GetMySyntheticField());
         }
 
-        public void M2() {
+        public void M2()
+        {
             var o = new object();
             SetMyNestedSyntheticField(o);
             Sink(GetMyNestedSyntheticField());
         }
 
-        public void M3() {
+        public void M3()
+        {
             var o = new object();
             SetMyFieldOnSyntheticField(o);
             Sink(GetMyFieldOnSyntheticField());
         }
 
         static void Sink(object o) { }
     }
+
+    // Test content data flow provenance.
+    public class L
+    {
+        public void M1()
+        {
+            var l = new Library();
+            var o = new object();
+            l.SetValue(o);
+            Sink(l.GetValue());
+        }
+
+        static void Sink(object o) { }
+    }
+
 }
diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs_ b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs_
@@ -13,5 +13,9 @@ namespace My.Qltest
         public static object StepArgReturnGenerated(object x) => throw null;
 
         public static object StepArgReturnGeneratedIgnored(object x) => throw null;
+
+        public void SetValue(object o) => throw null;
+
+        public object GetValue() => throw null;
     }
 }
diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.dll b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.dll
diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected
diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml
@@ -43,6 +43,8 @@ extensions:
       - ["My.Qltest", "K", false, "GetMyNestedSyntheticField", "()", "", "Argument[this].SyntheticField[My.Qltest.K.MySyntheticField1].SyntheticField[MySyntheticField1.MyNestedSyntheticField]", "ReturnValue", "value", "manual"]
       - ["My.Qltest", "K", false, "SetMyFieldOnSyntheticField", "(System.Object)", "", "Argument[0]", "Argument[this].SyntheticField[My.Qltest.K.MySyntheticField2].Field[My.Qltest.K.MyField]", "value", "manual"]
       - ["My.Qltest", "K", false, "GetMyFieldOnSyntheticField", "()", "", "Argument[this].SyntheticField[My.Qltest.K.MySyntheticField2].Field[My.Qltest.K.MyField]", "ReturnValue", "value", "manual"]
+      - ["My.Qltest", "Library", false, "SetValue", "(System.Object)", "", "Argument[0]", "Argument[this].SyntheticField[X]", "value", "dfc-generated"]
+      - ["My.Qltest", "Library", false, "GetValue", "()", "", "Argument[this].SyntheticField[X]", "ReturnValue", "value", "dfc-generated"]
 
   - addsTo:
       pack: codeql/csharp-all

diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/CaptureContentSummaryModels.ql b/csharp/ql/test/utils/modelgenerator/dataflow/CaptureContentSummaryModels.ql
@@ -3,7 +3,7 @@ import utils.modelgenerator.internal.CaptureModels
 import TestUtilities.InlineMadTest
 
 module InlineMadTestConfig implements InlineMadTestConfigSig {
-  string getCapturedModel(Callable c) { result = ContentSensitive::captureFlow(c) }
+  string getCapturedModel(Callable c) { result = ContentSensitive::captureFlow(c, _) }
 
   string getKind() { result = "contentbased-summary" }
 }

diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs b/csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs
diff --git a/java/ql/src/utils/modelgenerator/CaptureContentSummaryModels.ql b/java/ql/src/utils/modelgenerator/CaptureContentSummaryModels.ql
@@ -9,5 +9,5 @@
 import internal.CaptureModels
 
 from DataFlowSummaryTargetApi api, string flow
-where flow = ContentSensitive::captureFlow(api)
+where flow = ContentSensitive::captureFlow(api, _)
 select flow order by flow
diff --git a/java/ql/src/utils/modelgenerator/CaptureMixedNeutralModels.ql b/java/ql/src/utils/modelgenerator/CaptureMixedNeutralModels.ql
@@ -0,0 +1,13 @@
+/**
+ * @name Capture mixed neutral models.
+ * @description Finds neutral models to be used by other queries.
+ * @kind diagnostic
+ * @id java/utils/modelgenerator/mixed-neutral-models
+ * @tags modelgenerator
+ */
+
+import internal.CaptureModels
+
+from DataFlowSummaryTargetApi api, string noflow
+where noflow = captureMixedNeutral(api)
+select noflow order by noflow
diff --git a/java/ql/src/utils/modelgenerator/CaptureMixedSummaryModels.ql b/java/ql/src/utils/modelgenerator/CaptureMixedSummaryModels.ql
@@ -0,0 +1,13 @@
+/**
+ * @name Capture mixed summary models.
+ * @description Finds applicable summary models to be used by other queries.
+ * @kind diagnostic
+ * @id java/utils/modelgenerator/mixed-summary-models
+ * @tags modelgenerator
+ */
+
+import internal.CaptureModels
+
+from DataFlowSummaryTargetApi api, string flow
+where flow = captureMixedFlow(api, _)
+select flow order by flow
diff --git a/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll b/java/ql/src/utils/modelgenerator/internal/CaptureModels.qll
@@ -110,7 +110,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataF
 
     Callable lift() { result = lift }
 
-    predicate isRelevant() { relevant(this) }
+    predicate isRelevant() {
+      relevant(this) and
+      not hasManualSummaryModel(this)
+    }
   }
 
   private string isExtensible(Callable c) {

diff --git a/java/ql/test/library-tests/dataflow/external-models/C.java b/java/ql/test/library-tests/dataflow/external-models/C.java
@@ -54,6 +54,8 @@ void fooLibrary() {
     // The summary for the parameter is ignored, because it is generated and
     // because there is a manual neutral summary model for this callable.
     lib.apiStepArgQualGeneratedIgnored(arg1);
+
+    lib.getValue();
   }
 
   void fooPossibleLibraryDispatch(Library lib) {

diff --git a/java/ql/test/library-tests/dataflow/external-models/steps.expected b/java/ql/test/library-tests/dataflow/external-models/steps.expected
@@ -11,4 +11,5 @@ invalidModelRow
 | C.java:44:32:44:35 | arg1 | C.java:44:5:44:36 | apiStepArgResGenerated(...) |
 | C.java:50:45:50:48 | arg2 | C.java:50:5:50:49 | apiStepArgResGeneratedIgnored(...) |
 | C.java:52:33:52:36 | arg1 | C.java:52:5:52:7 | lib [post update] |
-| C.java:62:12:62:15 | arg1 | C.java:62:5:62:16 | id(...) |
+| C.java:58:5:58:7 | lib | C.java:58:5:58:18 | getValue(...) |
+| C.java:64:12:64:15 | arg1 | C.java:64:5:64:16 | id(...) |
diff --git a/java/ql/test/library-tests/dataflow/external-models/steps.ext.yml b/java/ql/test/library-tests/dataflow/external-models/steps.ext.yml
@@ -16,6 +16,7 @@ extensions:
       - ["my.qltest.external", "Library", False, "apiStepArgQualGenerated", "(Object)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"]
       - ["my.qltest.external", "Library", False, "apiStepArgQualGeneratedIgnored", "(Object)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"]
       - ["my.qltest.external", "Library", False, "id", "(Object)", "", "Argument[0]", "ReturnValue", "taint", "df-generated"]
+      - ["my.qltest.external", "Library", False, "getValue", "()", "", "Argument[this]", "ReturnValue", "taint", "dfc-generated"]
   - addsTo:
       pack: codeql/java-all
       extensible: neutralModel

diff --git a/java/ql/test/library-tests/dataflow/external-models/stubs/Library.java b/java/ql/test/library-tests/dataflow/external-models/stubs/Library.java
@@ -20,4 +20,8 @@ public Object apiStepArgQualGeneratedIgnored(Object x) {
   public Object id(Object x) {
     return null;
   }
+
+  public Object getValue() {
+    return null;
+  }
 }
diff --git a/java/ql/test/utils/modelgenerator/dataflow/CaptureContentSummaryModels.ql b/java/ql/test/utils/modelgenerator/dataflow/CaptureContentSummaryModels.ql
@@ -3,7 +3,7 @@ import utils.modelgenerator.internal.CaptureModels
 import TestUtilities.InlineMadTest
 
 module InlineMadTestConfig implements InlineMadTestConfigSig {
-  string getCapturedModel(Callable c) { result = ContentSensitive::captureFlow(c) }
+  string getCapturedModel(Callable c) { result = ContentSensitive::captureFlow(c, _) }
 
   string getKind() { result = "contentbased-summary" }
 }

diff --git a/java/ql/test/utils/modelgenerator/dataflow/p/Factory.java b/java/ql/test/utils/modelgenerator/dataflow/p/Factory.java
@@ -7,13 +7,13 @@ public final class Factory {
   private int intValue;
 
   // summary=p;Factory;false;create;(String,int);;Argument[0];ReturnValue;taint;df-generated
-  // contentbased-summary=p;Factory;false;create;(String,int);;Argument[0];ReturnValue.Field[p.Factory.value];value;df-generated
+  // contentbased-summary=p;Factory;false;create;(String,int);;Argument[0];ReturnValue.Field[p.Factory.value];value;dfc-generated
   public static Factory create(String value, int foo) {
     return new Factory(value, foo);
   }
 
   // summary=p;Factory;false;create;(String);;Argument[0];ReturnValue;taint;df-generated
-  // contentbased-summary=p;Factory;false;create;(String);;Argument[0];ReturnValue.Field[p.Factory.value];value;df-generated
+  // contentbased-summary=p;Factory;false;create;(String);;Argument[0];ReturnValue.Field[p.Factory.value];value;dfc-generated
   public static Factory create(String value) {
     return new Factory(value, 0);
   }
@@ -24,7 +24,7 @@ private Factory(String value, int intValue) {
   }
 
   // summary=p;Factory;false;getValue;();;Argument[this];ReturnValue;taint;df-generated
-  // contentbased-summary=p;Factory;false;getValue;();;Argument[this].Field[p.Factory.value];ReturnValue;value;df-generated
+  // contentbased-summary=p;Factory;false;getValue;();;Argument[this].Field[p.Factory.value];ReturnValue;value;dfc-generated
   public String getValue() {
     return value;
   }

diff --git a/java/ql/test/utils/modelgenerator/dataflow/p/Fanout.java b/java/ql/test/utils/modelgenerator/dataflow/p/Fanout.java
@@ -0,0 +1,68 @@
+package p;
+
+public class Fanout {
+  public interface I1 {
+    String getValue();
+  }
+
+  public interface I2 extends I1 {}
+
+  public class Impl1 implements I1 {
+    public String v;
+
+    // summary=p;Fanout$I1;true;getValue;();;Argument[this];ReturnValue;taint;df-generated
+    // contentbased-summary=p;Fanout$Impl1;true;getValue;();;Argument[this].Field[p.Fanout$Impl1.v];ReturnValue;value;dfc-generated
+    public String getValue() {
+      return v;
+    }
+  }
+
+  public class Impl2 implements I2 {
+    public String v;
+
+    // summary=p;Fanout$I1;true;getValue;();;Argument[this];ReturnValue;taint;df-generated
+    // contentbased-summary=p;Fanout$Impl2;true;getValue;();;Argument[this].Field[p.Fanout$Impl2.v];ReturnValue;value;dfc-generated
+    public String getValue() {
+      return v;
+    }
+  }
+
+  public class Impl3 implements I2 {
+    public String v;
+
+    // summary=p;Fanout$I1;true;getValue;();;Argument[this];ReturnValue;taint;df-generated
+    // contentbased-summary=p;Fanout$Impl3;true;getValue;();;Argument[this].Field[p.Fanout$Impl3.v];ReturnValue;value;dfc-generated
+    public String getValue() {
+      return v;
+    }
+  }
+
+  public class Impl4 implements I2 {
+    public String v;
+
+    // summary=p;Fanout$I1;true;getValue;();;Argument[this];ReturnValue;taint;df-generated
+    // contentbased-summary=p;Fanout$Impl4;true;getValue;();;Argument[this].Field[p.Fanout$Impl4.v];ReturnValue;value;dfc-generated
+    public String getValue() {
+      return v;
+    }
+  }
+
+  // summary=p;Fanout;true;concatGetValueOnI1;(String,Fanout$I1);;Argument[0];ReturnValue;taint;df-generated
+  // summary=p;Fanout;true;concatGetValueOnI1;(String,Fanout$I1);;Argument[1];ReturnValue;taint;df-generated
+  // No content based summaries are expected for this method on parameter `i`
+  // as the fanout (number of content flows) exceeds the limit of 3.
+  // contentbased-summary=p;Fanout;true;concatGetValueOnI1;(String,Fanout$I1);;Argument[0];ReturnValue;taint;dfc-generated
+  public String concatGetValueOnI1(String other, I1 i) {
+    return other + i.getValue();
+  }
+
+  // summary=p;Fanout;true;concatGetValueOnI2;(String,Fanout$I2);;Argument[0];ReturnValue;taint;df-generated
+  // summary=p;Fanout;true;concatGetValueOnI2;(String,Fanout$I2);;Argument[1];ReturnValue;taint;df-generated
+  // contentbased-summary=p;Fanout;true;concatGetValueOnI2;(String,Fanout$I2);;Argument[0];ReturnValue;taint;dfc-generated
+  // contentbased-summary=p;Fanout;true;concatGetValueOnI2;(String,Fanout$I2);;Argument[1].Field[p.Fanout$Impl2.v];ReturnValue;taint;dfc-generated
+  // contentbased-summary=p;Fanout;true;concatGetValueOnI2;(String,Fanout$I2);;Argument[1].Field[p.Fanout$Impl3.v];ReturnValue;taint;dfc-generated
+  // contentbased-summary=p;Fanout;true;concatGetValueOnI2;(String,Fanout$I2);;Argument[1].Field[p.Fanout$Impl4.v];ReturnValue;taint;dfc-generated
+  public String concatGetValueOnI2(String other, I2 i) {
+    return other + i.getValue();
+  }
+}
diff --git a/java/ql/test/utils/modelgenerator/dataflow/p/FinalClass.java b/java/ql/test/utils/modelgenerator/dataflow/p/FinalClass.java
@@ -5,7 +5,7 @@ public final class FinalClass {
   private static final String C = "constant";
 
   // summary=p;FinalClass;false;returnsInput;(String);;Argument[0];ReturnValue;taint;df-generated
-  // contentbased-summary=p;FinalClass;false;returnsInput;(String);;Argument[0];ReturnValue;value;df-generated
+  // contentbased-summary=p;FinalClass;false;returnsInput;(String);;Argument[0];ReturnValue;value;dfc-generated
   public String returnsInput(String input) {
     return input;
   }

diff --git a/java/ql/test/utils/modelgenerator/dataflow/p/FluentAPI.java b/java/ql/test/utils/modelgenerator/dataflow/p/FluentAPI.java
@@ -3,7 +3,7 @@
 public final class FluentAPI {
 
   // summary=p;FluentAPI;false;returnsThis;(String);;Argument[this];ReturnValue;value;df-generated
-  // contentbased-summary=p;FluentAPI;false;returnsThis;(String);;Argument[this];ReturnValue;value;df-generated
+  // contentbased-summary=p;FluentAPI;false;returnsThis;(String);;Argument[this];ReturnValue;value;dfc-generated
   public FluentAPI returnsThis(String input) {
     return this;
   }

diff --git a/java/ql/test/utils/modelgenerator/dataflow/p/ImmutablePojo.java b/java/ql/test/utils/modelgenerator/dataflow/p/ImmutablePojo.java
@@ -7,14 +7,14 @@ public final class ImmutablePojo {
   private final long x;
 
   // summary=p;ImmutablePojo;false;ImmutablePojo;(String,int);;Argument[0];Argument[this];taint;df-generated
-  // contentbased-summary=p;ImmutablePojo;false;ImmutablePojo;(String,int);;Argument[0];Argument[this].SyntheticField[p.ImmutablePojo.value];value;df-generated
+  // contentbased-summary=p;ImmutablePojo;false;ImmutablePojo;(String,int);;Argument[0];Argument[this].SyntheticField[p.ImmutablePojo.value];value;dfc-generated
   public ImmutablePojo(String value, int x) {
     this.value = value;
     this.x = x;
   }
 
   // summary=p;ImmutablePojo;false;getValue;();;Argument[this];ReturnValue;taint;df-generated
-  // contentbased-summary=p;ImmutablePojo;false;getValue;();;Argument[this].SyntheticField[p.ImmutablePojo.value];ReturnValue;value;df-generated
+  // contentbased-summary=p;ImmutablePojo;false;getValue;();;Argument[this].SyntheticField[p.ImmutablePojo.value];ReturnValue;value;dfc-generated
   public String getValue() {
     return value;
   }
@@ -26,8 +26,8 @@ public long getX() {
 
   // summary=p;ImmutablePojo;false;or;(String);;Argument[0];ReturnValue;taint;df-generated
   // summary=p;ImmutablePojo;false;or;(String);;Argument[this];ReturnValue;taint;df-generated
-  // contentbased-summary=p;ImmutablePojo;false;or;(String);;Argument[0];ReturnValue;value;df-generated
-  // contentbased-summary=p;ImmutablePojo;false;or;(String);;Argument[this].SyntheticField[p.ImmutablePojo.value];ReturnValue;value;df-generated
+  // contentbased-summary=p;ImmutablePojo;false;or;(String);;Argument[0];ReturnValue;value;dfc-generated
+  // contentbased-summary=p;ImmutablePojo;false;or;(String);;Argument[this].SyntheticField[p.ImmutablePojo.value];ReturnValue;value;dfc-generated
   public String or(String defaultValue) {
     return value != null ? value : defaultValue;
   }