Capture sources flowing into parameters

Author: Benjamin Muskalla

java/ql/src/utils/model-generator/CaptureSourceModels.ql

@@ -13,6 +13,7 @@ private import ModelGeneratorUtils
 private import semmle.code.java.dataflow.internal.FlowSummaryImplSpecific
 private import semmle.code.java.dataflow.internal.FlowSummaryImpl
 private import semmle.code.java.dataflow.internal.DataFlowImplCommon
+private import semmle.code.java.dataflow.internal.DataFlowPrivate
 private import semmle.code.java.dataflow.internal.DataFlowNodes::Private
 
 class FromSourceConfiguration extends TaintTracking::Configuration {
@@ -22,7 +23,7 @@ class FromSourceConfiguration extends TaintTracking::Configuration {
 
   override predicate isSink(DataFlow::Node sink) {
     exists(TargetAPI c |
-      sink instanceof ReturnNode and
+      sink instanceof ReturnNodeExt and
       sink.getEnclosingCallable() = c and
       c.isPublic() and
       c.fromSource()
@@ -32,14 +33,30 @@ class FromSourceConfiguration extends TaintTracking::Configuration {
   override DataFlow::FlowFeature getAFeature() {
     result instanceof DataFlow::FeatureHasSinkCallContext
   }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    exists(DataFlow::Content f |
+      readStep(node1, f, node2) and
+      if f instanceof DataFlow::FieldContent
+      then isRelevantType(f.(DataFlow::FieldContent).getField().getType())
+      else any()
+    )
+    or
+    exists(DataFlow::Content f | storeStep(node1, f, node2) |
+      f instanceof DataFlow::ArrayContent or
+      f instanceof DataFlow::CollectionContent or
+      f instanceof DataFlow::MapKeyContent or
+      f instanceof DataFlow::MapValueContent
+    )
+  }
 }
 
 string captureSource(TargetAPI api) {
   exists(DataFlow::Node source, DataFlow::Node sink, FromSourceConfiguration config, string kind |
     config.hasFlow(source, sink) and
     sourceNode(source, kind) and
     api = source.getEnclosingCallable() and
-    result = asSourceModel(api, "ReturnValue", kind)
+    result = asSourceModel(api, returnNodeAsOutput(api, sink), kind)
   )
 }
 

java/ql/src/utils/model-generator/CaptureSummaryModels.ql

@@ -103,17 +103,10 @@ string captureFieldFlow(TargetAPI api) {
     not api.getDeclaringType() instanceof EnumType and
     isRelevantType(returnNodeExt.getType())
   |
-    result = asTaintModel(api, "Argument[-1]", asOutput(api, returnNodeExt))
+    result = asTaintModel(api, "Argument[-1]", returnNodeAsOutput(api, returnNodeExt))
   )
 }
 
-string asOutput(TargetAPI api, ReturnNodeExt node) {
-  if node.getKind() instanceof ValueReturnKind
-  then result = "ReturnValue"
-  else
-    result = parameterAccess(api.getParameter(node.getKind().(ParamUpdateReturnKind).getPosition()))
-}
-
 class FieldAssignment extends AssignExpr {
   FieldAssignment() { exists(Field f | f.getAnAccess() = this.getDest()) }
 }
@@ -166,7 +159,6 @@ private predicate thisAccess(DataFlow::Node n) {
  */
 string captureFieldFlowIn(TargetAPI api) {
   exists(DataFlow::Node source, ParameterToFieldConfig config |
-    not api.isStatic() and
     config.hasFlow(source, _) and
     source.asParameter().getCallable() = api
   |
@@ -257,27 +249,6 @@ string captureParameterToParameterFlow(TargetAPI api) {
   )
 }
 
-predicate isRelevantType(Type t) {
-  not t instanceof TypeClass and
-  not t instanceof EnumType and
-  not t instanceof PrimitiveType and
-  not t instanceof BoxedType and
-  not t.(RefType).getAnAncestor().hasQualifiedName("java.lang", "Number") and
-  not t.(RefType).getAnAncestor().hasQualifiedName("java.nio.charset", "Charset") and
-  (
-    not t.(Array).getElementType() instanceof PrimitiveType or
-    isPrimitiveTypeUsedForBulkData(t.(Array).getElementType())
-  ) and
-  (
-    not t.(Array).getElementType() instanceof BoxedType or
-    isPrimitiveTypeUsedForBulkData(t.(Array).getElementType())
-  ) and
-  (
-    not t.(CollectionType).getElementType() instanceof BoxedType or
-    isPrimitiveTypeUsedForBulkData(t.(CollectionType).getElementType())
-  )
-}
-
 from TargetAPI api, string flow
 where flow = captureFlow(api)
 select flow order by flow

java/ql/src/utils/model-generator/ModelGeneratorUtils.qll

@@ -1,6 +1,7 @@
 import java
-import semmle.code.java.dataflow.ExternalFlow
-import semmle.code.java.dataflow.internal.ContainerFlow
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.internal.ContainerFlow
+private import semmle.code.java.dataflow.internal.DataFlowImplCommon
 
 Method superImpl(Method m) {
   result = m.getAnOverride() and
@@ -110,6 +111,34 @@ private string typeAsModel(RefType type) {
   result = type.getCompilationUnit().getPackage().getName() + ";" + type.nestedName()
 }
 
+predicate isRelevantType(Type t) {
+  not t instanceof TypeClass and
+  not t instanceof EnumType and
+  not t instanceof PrimitiveType and
+  not t instanceof BoxedType and
+  not t.(RefType).getAnAncestor().hasQualifiedName("java.lang", "Number") and
+  not t.(RefType).getAnAncestor().hasQualifiedName("java.nio.charset", "Charset") and
+  (
+    not t.(Array).getElementType() instanceof PrimitiveType or
+    isPrimitiveTypeUsedForBulkData(t.(Array).getElementType())
+  ) and
+  (
+    not t.(Array).getElementType() instanceof BoxedType or
+    isPrimitiveTypeUsedForBulkData(t.(Array).getElementType())
+  ) and
+  (
+    not t.(CollectionType).getElementType() instanceof BoxedType or
+    isPrimitiveTypeUsedForBulkData(t.(CollectionType).getElementType())
+  )
+}
+
+string returnNodeAsOutput(TargetAPI api, ReturnNodeExt node) {
+  if node.getKind() instanceof ValueReturnKind
+  then result = "ReturnValue"
+  else
+    result = parameterAccess(api.getParameter(node.getKind().(ParamUpdateReturnKind).getPosition()))
+}
+
 string parameterAccess(Parameter p) {
   if
     p.getType() instanceof Array and

java/ql/test/utils/model-generator/CaptureSourceModels.expected

@@ -1,2 +1,4 @@
 | p;Sources;true;readUrl;(URL);;ReturnValue;remote |
 | p;Sources;true;socketStream;();;ReturnValue;remote |
+| p;Sources;true;sourceToParameter;(InputStream[],List);;ArrayElement of Argument[0];remote |
+| p;Sources;true;sourceToParameter;(InputStream[],List);;Element of Argument[1];remote |

java/ql/test/utils/model-generator/p/Sources.java

@@ -5,6 +5,7 @@
 import java.net.ServerSocket;
 import java.net.URL;
 import java.util.function.Consumer;
+import java.util.List;
 
 
 public class Sources {
@@ -18,4 +19,10 @@ public InputStream socketStream() throws IOException {
         return socket.accept().getInputStream();
     }
 
+    public void sourceToParameter(InputStream[] streams, List<InputStream> otherStreams) throws IOException {
+        ServerSocket socket = new ServerSocket(123);
+        streams[0] = socket.accept().getInputStream();
+        otherStreams.add(socket.accept().getInputStream());
+    }
+
 }