Merge pull request #16701 from michaelnebel/csharp/modelgentaintmembers

C#/Java: Improve Sink and Summary model generation.

java/ql/src/utils/modelgenerator/internal/CaptureModels.qll

@@ -41,18 +41,21 @@ private module Printing implements PrintingSig {
 
 module ModelPrinting = PrintingImpl<Printing>;
 
+/**
+ * Holds if `c` is a relevant content kind, where the underlying type is relevant.
+ */
+private predicate isRelevantTypeInContent(DataFlow::Content c) {
+  isRelevantType(getUnderlyingContentType(c))
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` either via a read or a write of an intermediate field `f`.
  */
 private predicate isRelevantTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
   exists(DataFlow::Content f |
     DataFlowPrivate::readStep(node1, f, node2) and
-    if f instanceof DataFlow::FieldContent
-    then isRelevantType(f.(DataFlow::FieldContent).getField().getType())
-    else
-      if f instanceof DataFlow::SyntheticFieldContent
-      then isRelevantType(f.(DataFlow::SyntheticFieldContent).getField().getType())
-      else any()
+    // Partially restrict the content types used for intermediate steps.
+    (not exists(getUnderlyingContentType(f)) or isRelevantTypeInContent(f))
   )
   or
   exists(DataFlow::Content f | DataFlowPrivate::storeStep(node1, f, node2) |
@@ -61,12 +64,11 @@ private predicate isRelevantTaintStep(DataFlow::Node node1, DataFlow::Node node2
 }
 
 /**
- * Holds if content `c` is either a field or synthetic field of a relevant type
- * or a container like content.
+ * Holds if content `c` is either a field, a synthetic field or language specific
+ * content of a relevant type or a container like content.
  */
 private predicate isRelevantContent(DataFlow::Content c) {
-  isRelevantType(c.(DataFlow::FieldContent).getField().getType()) or
-  isRelevantType(c.(DataFlow::SyntheticFieldContent).getField().getType()) or
+  isRelevantTypeInContent(c) or
   DataFlowPrivate::containerContent(c)
 }
 
@@ -258,6 +260,10 @@ module PropagateToSinkConfig implements DataFlow::ConfigSig {
   predicate isBarrier(DataFlow::Node node) { sinkModelSanitizer(node) }
 
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
+
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isRelevantTaintStep(node1, node2)
+  }
 }
 
 private module PropagateToSink = TaintTracking::Global<PropagateToSinkConfig>;

java/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll

@@ -186,6 +186,14 @@ predicate isRelevantType(J::Type t) {
   )
 }
 
+/**
+ * Gets the underlying type of the content `c`.
+ */
+J::Type getUnderlyingContentType(DataFlow::Content c) {
+  result = c.(DataFlow::FieldContent).getField().getType() or
+  result = c.(DataFlow::SyntheticFieldContent).getField().getType()
+}
+
 /**
  * Gets the MaD string representation of the qualifier.
  */

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

@@ -0,0 +1,13 @@
+extensions:
+
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - [ "p", "Sinks", False, "sink", "(Object)", "", "Argument[0]", "test-sink", "manual" ]
+
+  - addsTo:
+      pack: codeql/java-all
+      extensible: neutralModel
+    data:
+      - [ "p", "Sinks", "hasManualSinkNeutral", "(Object)", "sink", "manual"]

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

@@ -11,6 +11,12 @@ import java.util.logging.Logger;
 
 public class Sinks {
 
+  public Object tainted;
+
+  // Defined as a sink in the model file next to the test.
+  // neutral=p;Sinks;sink;(Object);summary;df-generated
+  public void sink(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
@@ -38,4 +44,24 @@ public class Sinks {
     Logger logger = Logger.getLogger(Sinks.class.getSimpleName());
     logger.warning(s);
   }
+
+  // New sink as the value of a public field is propagated to a sink.
+  // sink=p;Sinks;true;fieldSink;();;Argument[this];test-sink;df-generated
+  // neutral=p;Sinks;fieldSink;();summary;df-generated
+  public void fieldSink() {
+    sink(tainted);
+  }
+
+  // Not a new sink as this method is already defined as a manual
+  // sink neutral.
+  // neutral=p;Sinks;hasManualSinkNeutral;(Object);summary;df-generated
+  public void hasManualSinkNeutral(Object o) {
+    sink(o);
+  }
+
+  // sink=p;Sinks;true;compoundPropgate;(Sinks);;Argument[0];test-sink;df-generated
+  // neutral=p;Sinks;compoundPropgate;(Sinks);summary;df-generated
+  public void compoundPropgate(Sinks s) {
+    s.fieldSink();
+  }
 }