Merge pull request github#7832 from aschackmull/java/modelgen

Java: Simplify model generator query using flow state.

Author: michaelnebel

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

@@ -43,8 +43,8 @@ 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, returnNodeAsOutput(api, sink), kind)
+    api = sink.getEnclosingCallable() and
+    result = asSourceModel(api, returnNodeAsOutput(sink), kind)
   )
 }
 

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

@@ -14,10 +14,7 @@ import ModelGeneratorUtils
 
 string captureFlow(TargetAPI api) {
   result = captureQualifierFlow(api) or
-  result = captureParameterFlowToReturnValue(api) or
-  result = captureFieldFlowIn(api) or
-  result = captureParameterToParameterFlow(api) or
-  result = captureFieldFlow(api)
+  result = captureThroughFlow(api)
 }
 
 /**
@@ -40,21 +37,51 @@ string captureQualifierFlow(TargetAPI api) {
   result = asValueModel(api, "Argument[-1]", "ReturnValue")
 }
 
-class FieldToReturnConfig extends TaintTracking::Configuration {
-  FieldToReturnConfig() { this = "FieldToReturnConfig" }
+class TaintRead extends DataFlow::FlowState {
+  TaintRead() { this = "TaintRead" }
+}
+
+class TaintStore extends DataFlow::FlowState {
+  TaintStore() { this = "TaintStore" }
+}
 
-  override predicate isSource(DataFlow::Node source) {
-    source instanceof DataFlow::InstanceParameterNode
+class ThroughFlowConfig extends TaintTracking::Configuration {
+  ThroughFlowConfig() { this = "ThroughFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
+    source instanceof DataFlow::ParameterNode and
+    source.getEnclosingCallable() instanceof TargetAPI and
+    state instanceof TaintRead
   }
 
-  override predicate isSink(DataFlow::Node sink) {
+  override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
     sink instanceof ReturnNodeExt and
     not sink.(ReturnNode).asExpr().(ThisAccess).isOwnInstanceAccess() and
-    not exists(captureQualifierFlow(sink.asExpr().getEnclosingCallable()))
+    not exists(captureQualifierFlow(sink.asExpr().getEnclosingCallable())) and
+    (state instanceof TaintRead or state instanceof TaintStore)
+  }
+
+  override predicate isAdditionalFlowStep(
+    DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
+    DataFlow::FlowState state2
+  ) {
+    exists(TypedContent tc |
+      store(node1, tc, node2, _) and
+      isRelevantContent(tc.getContent()) and
+      (state1 instanceof TaintRead or state1 instanceof TaintStore) and
+      state2 instanceof TaintStore
+    )
+    or
+    exists(DataFlow::Content c |
+      readStep(node1, c, node2) and
+      isRelevantContent(c) and
+      state1 instanceof TaintRead and
+      state2 instanceof TaintRead
+    )
   }
 
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    isRelevantTaintStep(node1, node2)
+  override predicate isSanitizer(DataFlow::Node n) {
+    exists(Type t | t = n.getType() and not isRelevantType(t))
   }
 
   override DataFlow::FlowFeature getAFeature() {
@@ -63,8 +90,12 @@ class FieldToReturnConfig extends TaintTracking::Configuration {
 }
 
 /**
- * Capture APIs that return tainted instance data.
- * Example of an API that returns tainted instance data:
+ * Capture APIs that transfer taint from an input parameter to an output return
+ * value or parameter.
+ * Allows a sequence of read steps followed by a sequence of store steps.
+ *
+ * Examples:
+ *
  * ```
  * public class Foo {
  *   private String tainted;
@@ -83,48 +114,7 @@ class FieldToReturnConfig extends TaintTracking::Configuration {
  * p;Foo;true;returnsTainted;;Argument[-1];ReturnValue;taint
  * p;Foo;true;putsTaintIntoParameter;(List);Argument[-1];Argument[0];taint
  * ```
- */
-string captureFieldFlow(TargetAPI api) {
-  exists(FieldToReturnConfig config, ReturnNodeExt returnNodeExt |
-    config.hasFlow(_, returnNodeExt) and
-    returnNodeExt.getEnclosingCallable() = api and
-    not api.getDeclaringType() instanceof EnumType and
-    isRelevantType(returnNodeExt.getType())
-  |
-    result = asTaintModel(api, "Argument[-1]", returnNodeAsOutput(api, returnNodeExt))
-  )
-}
-
-class ParameterToFieldConfig extends TaintTracking::Configuration {
-  ParameterToFieldConfig() { this = "ParameterToFieldConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
-    source instanceof DataFlow::ParameterNode and
-    isRelevantType(source.getType())
-  }
-
-  override predicate isSink(DataFlow::Node sink) {
-    thisAccess(sink.(DataFlow::PostUpdateNode).getPreUpdateNode())
-  }
-
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    store(node1, _, node2, _)
-  }
-
-  override DataFlow::FlowFeature getAFeature() {
-    result instanceof DataFlow::FeatureEqualSourceSinkCallContext
-  }
-}
-
-private predicate thisAccess(DataFlow::Node n) {
-  n.asExpr().(InstanceAccess).isOwnInstanceAccess()
-  or
-  n.(DataFlow::ImplicitInstanceAccess).getInstanceAccess() instanceof OwnInstanceAccess
-}
-
-/**
- * Captures APIs that accept input and store them in a field.
- * Example:
+ *
  * ```
  * public class Foo {
  *  private String tainted;
@@ -134,96 +124,38 @@ private predicate thisAccess(DataFlow::Node n) {
  * ```
  * Captured Model:
  * `p;Foo;true;doSomething;(String);Argument[0];Argument[-1];taint`
- */
-string captureFieldFlowIn(TargetAPI api) {
-  exists(DataFlow::Node source, ParameterToFieldConfig config |
-    config.hasFlow(source, _) and
-    source.asParameter().getCallable() = api
-  |
-    result =
-      asTaintModel(api, "Argument[" + source.asParameter().getPosition() + "]", "Argument[-1]")
-  )
-}
-
-class ParameterToReturnValueTaintConfig extends TaintTracking::Configuration {
-  ParameterToReturnValueTaintConfig() { this = "ParameterToReturnValueTaintConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
-    exists(TargetAPI api |
-      api = source.asParameter().getCallable() and
-      isRelevantType(api.getReturnType()) and
-      isRelevantType(source.asParameter().getType())
-    )
-  }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof ReturnNode }
-
-  // consider store steps to track taint across objects to model factory methods returning tainted objects
-  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-    store(node1, _, node2, _)
-  }
-
-  override DataFlow::FlowFeature getAFeature() {
-    result instanceof DataFlow::FeatureEqualSourceSinkCallContext
-  }
-}
-
-predicate paramFlowToReturnValueExists(Parameter p) {
-  exists(ParameterToReturnValueTaintConfig config, ReturnStmt rtn |
-    config.hasFlow(DataFlow::parameterNode(p), DataFlow::exprNode(rtn.getResult()))
-  )
-}
-
-/**
- * Capture APIs that return (parts of) data passed in as a parameter.
- * Example:
+ *
  * ```
  * public class Foo {
- *
  *   public String returnData(String tainted) {
  *     return tainted.substring(0,10)
  *   }
  * }
  * ```
  * Captured Model:
- * ```
- * p;Foo;true;returnData;;Argument[0];ReturnValue;taint
- * ```
- */
-string captureParameterFlowToReturnValue(TargetAPI api) {
-  exists(Parameter p |
-    p = api.getAParameter() and
-    paramFlowToReturnValueExists(p)
-  |
-    result = asTaintModel(api, parameterAccess(p), "ReturnValue")
-  )
-}
-
-/**
- * Capture APIs that pass tainted data from a parameter to a parameter.
- * Example:
+ * `p;Foo;true;returnData;;Argument[0];ReturnValue;taint`
+ *
  * ```
  * public class Foo {
- *
  *   public void addToList(String tainted, List<String> foo) {
  *     foo.add(tainted);
  *   }
  * }
  * ```
  * Captured Model:
- * ```
- * p;Foo;true;addToList;;Argument[0];Argument[1];taint
- * ```
+ * `p;Foo;true;addToList;;Argument[0];Argument[1];taint`
  */
-string captureParameterToParameterFlow(TargetAPI api) {
-  exists(DataFlow::ParameterNode source, DataFlow::PostUpdateNode sink |
-    source.getEnclosingCallable() = api and
-    sink.getPreUpdateNode().asExpr() = api.getAParameter().getAnAccess() and
-    TaintTracking::localTaint(source, sink)
+string captureThroughFlow(TargetAPI api) {
+  exists(
+    ThroughFlowConfig config, DataFlow::ParameterNode p, ReturnNodeExt returnNodeExt, string input,
+    string output
   |
-    result =
-      asTaintModel(api, parameterAccess(source.asParameter()),
-        parameterAccess(sink.getPreUpdateNode().asExpr().(VarAccess).getVariable()))
+    config.hasFlow(p, returnNodeExt) and
+    returnNodeExt.getEnclosingCallable() = api and
+    input = parameterNodeAsInput(p) and
+    output = returnNodeAsOutput(returnNodeExt) and
+    input != output and
+    result = asTaintModel(api, input, output)
   )
 }
 

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

@@ -140,7 +140,10 @@ predicate isRelevantTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
     readStep(node1, f, node2) and
     if f instanceof DataFlow::FieldContent
     then isRelevantType(f.(DataFlow::FieldContent).getField().getType())
-    else any()
+    else
+      if f instanceof DataFlow::SyntheticFieldContent
+      then isRelevantType(f.(DataFlow::SyntheticFieldContent).getField().getType())
+      else any()
   )
   or
   exists(DataFlow::Content f | storeStep(node1, f, node2) |
@@ -151,11 +154,29 @@ predicate isRelevantTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
   )
 }
 
-string returnNodeAsOutput(TargetAPI api, ReturnNodeExt node) {
+predicate isRelevantContent(DataFlow::Content f) {
+  isRelevantType(f.(DataFlow::FieldContent).getField().getType()) or
+  f instanceof DataFlow::ArrayContent or
+  f instanceof DataFlow::CollectionContent or
+  f instanceof DataFlow::MapKeyContent or
+  f instanceof DataFlow::MapValueContent
+}
+
+string parameterNodeAsInput(DataFlow::ParameterNode p) {
+  result = parameterAccess(p.asParameter())
+  or
+  result = "Argument[-1]" and p instanceof DataFlow::InstanceParameterNode
+}
+
+string returnNodeAsOutput(ReturnNodeExt node) {
   if node.getKind() instanceof ValueReturnKind
   then result = "ReturnValue"
   else
-    result = parameterAccess(api.getParameter(node.getKind().(ParamUpdateReturnKind).getPosition()))
+    exists(int pos | pos = node.getKind().(ParamUpdateReturnKind).getPosition() |
+      result = parameterAccess(node.getEnclosingCallable().getParameter(pos))
+      or
+      result = "Argument[-1]" and pos = -1
+    )
 }
 
 string parameterAccess(Parameter p) {

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

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

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

@@ -21,7 +21,6 @@
 | p;Joiner;false;merge;(Joiner);;Argument[-1];ReturnValue;value |
 | p;Joiner;false;setEmptyValue;(CharSequence);;Argument[-1];ReturnValue;value |
 | p;Joiner;false;setEmptyValue;(CharSequence);;Argument[0];Argument[-1];taint |
-| p;Joiner;false;setEmptyValue;(CharSequence);;Argument[0];ReturnValue;taint |
 | p;Joiner;false;toString;();;Argument[-1];ReturnValue;taint |
 | p;MultipleImpls$Strat2;true;getValue;();;Argument[-1];ReturnValue;taint |
 | p;MultipleImpls$Strategy;true;doSomething;(String);;Argument[0];Argument[-1];taint |

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

@@ -19,6 +19,10 @@ public InputStream socketStream() throws IOException {
         return socket.accept().getInputStream();
     }
 
+    public InputStream wrappedSocketStream() throws IOException {
+        return socketStream();
+    }
+
     public void sourceToParameter(InputStream[] streams, List<InputStream> otherStreams) throws IOException {
         ServerSocket socket = new ServerSocket(123);
         streams[0] = socket.accept().getInputStream();