Merge pull request #16759 from michaelnebel/modelgen/sourcesinkmodelgen

C#/Java: Introduce source and sink model generation sanitisers.

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

@@ -145,7 +145,7 @@ private class TaintStore extends TaintState, TTaintStore {
  *
  * This can be used to generate Flow summaries for APIs from parameter to return.
  */
-module ThroughFlowConfig implements DataFlow::StateConfigSig {
+module PropagateFlowConfig implements DataFlow::StateConfigSig {
   class FlowState = TaintState;
 
   predicate isSource(DataFlow::Node source, FlowState state) {
@@ -190,14 +190,14 @@ module ThroughFlowConfig implements DataFlow::StateConfigSig {
   }
 }
 
-private module ThroughFlow = TaintTracking::GlobalWithState<ThroughFlowConfig>;
+private module PropagateFlow = TaintTracking::GlobalWithState<PropagateFlowConfig>;
 
 /**
  * Gets the summary model(s) of `api`, if there is flow from parameters to return value or parameter.
  */
 string captureThroughFlow(DataFlowTargetApi api) {
   exists(DataFlow::ParameterNode p, ReturnNodeExt returnNodeExt, string input, string output |
-    ThroughFlow::flow(p, returnNodeExt) and
+    PropagateFlow::flow(p, returnNodeExt) and
     returnNodeExt.(DataFlow::Node).getEnclosingCallable() = api and
     input = parameterNodeAsInput(p) and
     output = returnNodeExt.getOutput() and
@@ -213,8 +213,13 @@ string captureThroughFlow(DataFlowTargetApi api) {
  * This can be used to generate Source summaries for an API, if the API expose an already known source
  * via its return (then the API itself becomes a source).
  */
-module FromSourceConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { ExternalFlow::sourceNode(source, _) }
+module PropagateFromSourceConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(string kind |
+      isRelevantSourceKind(kind) and
+      ExternalFlow::sourceNode(source, kind)
+    )
+  }
 
   predicate isSink(DataFlow::Node sink) {
     exists(DataFlowTargetApi c |
@@ -225,22 +230,26 @@ module FromSourceConfig implements DataFlow::ConfigSig {
 
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSinkCallContext }
 
+  predicate isBarrier(DataFlow::Node n) {
+    exists(Type t | t = n.getType() and not isRelevantType(t))
+  }
+
   predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     isRelevantTaintStep(node1, node2)
   }
 }
 
-private module FromSource = TaintTracking::Global<FromSourceConfig>;
+private module PropagateFromSource = TaintTracking::Global<PropagateFromSourceConfig>;
 
 /**
  * Gets the source model(s) of `api`, if there is flow from an existing known source to the return of `api`.
  */
 string captureSource(DataFlowTargetApi api) {
   exists(DataFlow::Node source, ReturnNodeExt sink, string kind |
-    FromSource::flow(source, sink) and
+    PropagateFromSource::flow(source, sink) and
     ExternalFlow::sourceNode(source, kind) and
     api = sink.getEnclosingCallable() and
-    isRelevantSourceKind(kind) and
+    not irrelevantSourceSinkApi(source.getEnclosingCallable(), api) and
     result = ModelPrinting::asSourceModel(api, sink.getOutput(), kind)
   )
 }
@@ -255,9 +264,15 @@ string captureSource(DataFlowTargetApi api) {
 module PropagateToSinkConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { apiSource(source) }
 
-  predicate isSink(DataFlow::Node sink) { ExternalFlow::sinkNode(sink, _) }
+  predicate isSink(DataFlow::Node sink) {
+    exists(string kind | isRelevantSinkKind(kind) and ExternalFlow::sinkNode(sink, kind))
+  }
 
-  predicate isBarrier(DataFlow::Node node) { sinkModelSanitizer(node) }
+  predicate isBarrier(DataFlow::Node node) {
+    exists(Type t | t = node.getType() and not isRelevantType(t))
+    or
+    sinkModelSanitizer(node)
+  }
 
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
 
@@ -276,7 +291,6 @@ string captureSink(DataFlowTargetApi api) {
     PropagateToSink::flow(src, sink) and
     ExternalFlow::sinkNode(sink, kind) and
     api = src.getEnclosingCallable() and
-    isRelevantSinkKind(kind) and
     result = ModelPrinting::asSinkModel(api, asInputArgument(src), kind)
   )
 }

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

@@ -257,6 +257,29 @@ predicate apiSource(DataFlow::Node source) {
   )
 }
 
+private predicate uniquelyCalls(DataFlowCallable dc1, DataFlowCallable dc2) {
+  exists(DataFlowCall call |
+    dc1 = call.getEnclosingCallable() and
+    dc2 = unique(DataFlowCallable dc0 | dc0 = viableCallable(call) | dc0)
+  )
+}
+
+bindingset[dc1, dc2]
+private predicate uniquelyCallsPlus(DataFlowCallable dc1, DataFlowCallable dc2) =
+  fastTC(uniquelyCalls/2)(dc1, dc2)
+
+/**
+ * Holds if it is not relevant to generate a source model for `api`, even
+ * if flow is detected from a node within `source` to a sink within `api`.
+ */
+bindingset[sourceEnclosing, api]
+predicate irrelevantSourceSinkApi(Callable sourceEnclosing, TargetApiSpecific api) {
+  not exists(DataFlowCallable dc1, DataFlowCallable dc2 | uniquelyCallsPlus(dc1, dc2) or dc1 = dc2 |
+    dc1.getUnderlyingCallable() = api and
+    dc2.getUnderlyingCallable() = sourceEnclosing
+  )
+}
+
 /**
  * Gets the MaD input string representation of `source`.
  */

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

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/csharp-all
+      extensible: sinkModel
+    data:
+      - [ "Sinks", "NewSinks", False, "Sink", "(System.Object)", "", "Argument[0]", "test-sink", "manual"]

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

@@ -12,6 +12,10 @@ public class NewSinks
     public string TaintedProp { get; set; }
     public string PrivateSetTaintedProp { get; private set; }
 
+    // Sink defined in the extensible file next to the test.
+    // neutral=Sinks;NewSinks;Sink;(System.Object);summary;df-generated
+    public void Sink(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
@@ -78,6 +82,14 @@ public class NewSinks
         var response = new HttpResponse();
         response.WriteFile(PrivateSetTaintedProp);
     }
+
+    // Not a new sink because a simple type is used in an intermediate step
+    // neutral=Sinks;NewSinks;WrapResponseWriteFileSimpleType;(System.String);summary;df-generated
+    public void WrapResponseWriteFileSimpleType(string s)
+    {
+        var r = s == "hello";
+        Sink(r);
+    }
 }
 
 public class CompoundSinks

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

@@ -34,4 +34,34 @@ public class NewSources
     {
         return Console.ReadKey();
     }
+
+    // Not a new source because a simple type is used in an intermediate step
+    // neutral=Sources;NewSources;WrapConsoleReadLineGetBool;();summary;df-generated
+    public bool WrapConsoleReadLineGetBool()
+    {
+        var s = Console.ReadLine();
+        return s == "hello";
+    }
+
+    public class MyConsoleReader
+    {
+        // source=Sources;NewSources+MyConsoleReader;false;ToString;();;ReturnValue;local;df-generated
+        // neutral=Sources;NewSources+MyConsoleReader;ToString;();summary;df-generated
+        public override string ToString()
+        {
+            return Console.ReadLine();
+        }
+    }
+
+
+    public class MyContainer<T>
+    {
+        public T Value { get; set; }
+
+        // summary=Sources;NewSources+MyContainer<T>;false;Read;();;Argument[this];ReturnValue;taint;df-generated
+        public string Read()
+        {
+            return Value.ToString();
+        }
+    }
 }

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

@@ -145,7 +145,7 @@ private class TaintStore extends TaintState, TTaintStore {
  *
  * This can be used to generate Flow summaries for APIs from parameter to return.
  */
-module ThroughFlowConfig implements DataFlow::StateConfigSig {
+module PropagateFlowConfig implements DataFlow::StateConfigSig {
   class FlowState = TaintState;
 
   predicate isSource(DataFlow::Node source, FlowState state) {
@@ -190,14 +190,14 @@ module ThroughFlowConfig implements DataFlow::StateConfigSig {
   }
 }
 
-private module ThroughFlow = TaintTracking::GlobalWithState<ThroughFlowConfig>;
+private module PropagateFlow = TaintTracking::GlobalWithState<PropagateFlowConfig>;
 
 /**
  * Gets the summary model(s) of `api`, if there is flow from parameters to return value or parameter.
  */
 string captureThroughFlow(DataFlowTargetApi api) {
   exists(DataFlow::ParameterNode p, ReturnNodeExt returnNodeExt, string input, string output |
-    ThroughFlow::flow(p, returnNodeExt) and
+    PropagateFlow::flow(p, returnNodeExt) and
     returnNodeExt.(DataFlow::Node).getEnclosingCallable() = api and
     input = parameterNodeAsInput(p) and
     output = returnNodeExt.getOutput() and
@@ -213,8 +213,13 @@ string captureThroughFlow(DataFlowTargetApi api) {
  * This can be used to generate Source summaries for an API, if the API expose an already known source
  * via its return (then the API itself becomes a source).
  */
-module FromSourceConfig implements DataFlow::ConfigSig {
-  predicate isSource(DataFlow::Node source) { ExternalFlow::sourceNode(source, _) }
+module PropagateFromSourceConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(string kind |
+      isRelevantSourceKind(kind) and
+      ExternalFlow::sourceNode(source, kind)
+    )
+  }
 
   predicate isSink(DataFlow::Node sink) {
     exists(DataFlowTargetApi c |
@@ -225,22 +230,26 @@ module FromSourceConfig implements DataFlow::ConfigSig {
 
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSinkCallContext }
 
+  predicate isBarrier(DataFlow::Node n) {
+    exists(Type t | t = n.getType() and not isRelevantType(t))
+  }
+
   predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     isRelevantTaintStep(node1, node2)
   }
 }
 
-private module FromSource = TaintTracking::Global<FromSourceConfig>;
+private module PropagateFromSource = TaintTracking::Global<PropagateFromSourceConfig>;
 
 /**
  * Gets the source model(s) of `api`, if there is flow from an existing known source to the return of `api`.
  */
 string captureSource(DataFlowTargetApi api) {
   exists(DataFlow::Node source, ReturnNodeExt sink, string kind |
-    FromSource::flow(source, sink) and
+    PropagateFromSource::flow(source, sink) and
     ExternalFlow::sourceNode(source, kind) and
     api = sink.getEnclosingCallable() and
-    isRelevantSourceKind(kind) and
+    not irrelevantSourceSinkApi(source.getEnclosingCallable(), api) and
     result = ModelPrinting::asSourceModel(api, sink.getOutput(), kind)
   )
 }
@@ -255,9 +264,15 @@ string captureSource(DataFlowTargetApi api) {
 module PropagateToSinkConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { apiSource(source) }
 
-  predicate isSink(DataFlow::Node sink) { ExternalFlow::sinkNode(sink, _) }
+  predicate isSink(DataFlow::Node sink) {
+    exists(string kind | isRelevantSinkKind(kind) and ExternalFlow::sinkNode(sink, kind))
+  }
 
-  predicate isBarrier(DataFlow::Node node) { sinkModelSanitizer(node) }
+  predicate isBarrier(DataFlow::Node node) {
+    exists(Type t | t = node.getType() and not isRelevantType(t))
+    or
+    sinkModelSanitizer(node)
+  }
 
   DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
 
@@ -276,7 +291,6 @@ string captureSink(DataFlowTargetApi api) {
     PropagateToSink::flow(src, sink) and
     ExternalFlow::sinkNode(sink, kind) and
     api = src.getEnclosingCallable() and
-    isRelevantSinkKind(kind) and
     result = ModelPrinting::asSinkModel(api, asInputArgument(src), kind)
   )
 }

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

@@ -278,6 +278,12 @@ predicate apiSource(DataFlow::Node source) {
   )
 }
 
+/**
+ * Holds if it is not relevant to generate a source model for `api`, even
+ * if flow is detected from a node within `source` to a sink within `api`.
+ */
+predicate irrelevantSourceSinkApi(Callable source, TargetApiSpecific api) { none() }
+
 /**
  * Gets the MaD input string representation of `source`.
  */

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

@@ -0,0 +1,7 @@
+extensions:
+
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sourceModel
+    data:
+      - [ "p", "Sources", False, "source", "()", "", "ReturnValue", "test-source", "manual" ]

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

@@ -64,4 +64,11 @@ public class Sinks {
   public void compoundPropgate(Sinks s) {
     s.fieldSink();
   }
+
+  // Not a new sink because a simple type is used in an intermediate step
+  // neutral=p;Sinks;wrapSinkSimpleType;(String);summary;df-generated
+  public void wrapSinkSimpleType(String s) {
+    Boolean b = s == "hello";
+    sink(b);
+  }
 }

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

@@ -8,6 +8,12 @@ import java.util.List;
 
 public class Sources {
 
+  // Defined as a source in the model file next to the test.
+  // neutral=p;Sources;source;();summary;df-generated
+  public String source() {
+    return "";
+  }
+
   // source=p;Sources;true;readUrl;(URL);;ReturnValue;remote;df-generated
   // sink=p;Sources;true;readUrl;(URL);;Argument[0];request-forgery;df-generated
   // neutral=p;Sources;readUrl;(URL);summary;df-generated
@@ -37,4 +43,27 @@ public class Sources {
     streams[0] = socket.accept().getInputStream();
     otherStreams.add(socket.accept().getInputStream());
   }
+
+  // Not a new source because a simple type is used in an intermediate step
+  // neutral=p;Sources;wrapSourceGetBool;();summary;df-generated
+  public Boolean wrapSourceGetBool() {
+    String s = source();
+    return s == "hello";
+  }
+
+  public class SourceReader {
+    @Override
+    public String toString() {
+      return source();
+    }
+  }
+
+  public class MyContainer<T> {
+    private T value;
+
+    // neutral=p;Sources$MyContainer;read;();summary;df-generated
+    public String read() {
+      return value.toString();
+    }
+  }
 }