Merge pull request #6107 from aschackmull/dataflow/implicit-reads

Dataflow: Add support for implicit reads

java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

@@ -561,6 +561,7 @@ module CsvValidation {
   }
 }
 
+pragma[nomagic]
 private predicate elementSpec(
   string namespace, string type, boolean subtypes, string name, string signature, string ext
 ) {

java/ql/src/semmle/code/java/dataflow/FlowSummary.qll

@@ -5,7 +5,7 @@
 import java
 private import internal.FlowSummaryImpl as Impl
 private import internal.DataFlowDispatch
-private import internal.DataFlowPrivate
+private import internal.DataFlowUtil
 
 // import all instances of SummarizedCallable below
 private module Summaries {

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll

@@ -724,7 +724,6 @@ private module Cached {
     Node node1, Content c, Node node2, DataFlowType contentType, DataFlowType containerType
   ) {
     storeStep(node1, c, node2) and
-    read(_, c, _) and
     contentType = getNodeDataFlowType(node1) and
     containerType = getNodeDataFlowType(node2)
     or

java/ql/src/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

@@ -83,56 +83,6 @@ private predicate instanceFieldAssign(Expr src, FieldAccess fa) {
   )
 }
 
-private newtype TContent =
-  TFieldContent(InstanceField f) or
-  TArrayContent() or
-  TCollectionContent() or
-  TMapKeyContent() or
-  TMapValueContent()
-
-/**
- * A reference contained in an object. Examples include instance fields, the
- * contents of a collection object, or the contents of an array.
- */
-class Content extends TContent {
-  /** Gets a textual representation of this element. */
-  abstract string toString();
-
-  predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) {
-    path = "" and sl = 0 and sc = 0 and el = 0 and ec = 0
-  }
-}
-
-class FieldContent extends Content, TFieldContent {
-  InstanceField f;
-
-  FieldContent() { this = TFieldContent(f) }
-
-  InstanceField getField() { result = f }
-
-  override string toString() { result = f.toString() }
-
-  override predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) {
-    f.getLocation().hasLocationInfo(path, sl, sc, el, ec)
-  }
-}
-
-class ArrayContent extends Content, TArrayContent {
-  override string toString() { result = "[]" }
-}
-
-class CollectionContent extends Content, TCollectionContent {
-  override string toString() { result = "<element>" }
-}
-
-class MapKeyContent extends Content, TMapKeyContent {
-  override string toString() { result = "<map.key>" }
-}
-
-class MapValueContent extends Content, TMapValueContent {
-  override string toString() { result = "<map.value>" }
-}
-
 /**
  * Holds if data can flow from `node1` to `node2` via an assignment to `f`.
  * Thus, `node2` references an object with a field `f` that contains the

java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll

@@ -157,6 +157,62 @@ predicate simpleLocalFlowStep(Node node1, Node node2) {
   FlowSummaryImpl::Private::Steps::summaryLocalStep(node1, node2, true)
 }
 
+private newtype TContent =
+  TFieldContent(InstanceField f) or
+  TArrayContent() or
+  TCollectionContent() or
+  TMapKeyContent() or
+  TMapValueContent()
+
+/**
+ * A description of the way data may be stored inside an object. Examples
+ * include instance fields, the contents of a collection object, or the contents
+ * of an array.
+ */
+class Content extends TContent {
+  /** Gets a textual representation of this element. */
+  abstract string toString();
+
+  predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) {
+    path = "" and sl = 0 and sc = 0 and el = 0 and ec = 0
+  }
+}
+
+/** A reference through an instance field. */
+class FieldContent extends Content, TFieldContent {
+  InstanceField f;
+
+  FieldContent() { this = TFieldContent(f) }
+
+  InstanceField getField() { result = f }
+
+  override string toString() { result = f.toString() }
+
+  override predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) {
+    f.getLocation().hasLocationInfo(path, sl, sc, el, ec)
+  }
+}
+
+/** A reference through an array. */
+class ArrayContent extends Content, TArrayContent {
+  override string toString() { result = "[]" }
+}
+
+/** A reference through the contents of some collection-like container. */
+class CollectionContent extends Content, TCollectionContent {
+  override string toString() { result = "<element>" }
+}
+
+/** A reference through a map key. */
+class MapKeyContent extends Content, TMapKeyContent {
+  override string toString() { result = "<map.key>" }
+}
+
+/** A reference through a map value. */
+class MapValueContent extends Content, TMapValueContent {
+  override string toString() { result = "<map.value>" }
+}
+
 /**
  * A guard that validates some expression.
  *

java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

@@ -61,16 +61,16 @@ private module Cached {
     localAdditionalTaintUpdateStep(src.asExpr(),
       sink.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr())
     or
-    exists(Content f |
+    exists(DataFlow::Content f |
       readStep(src, f, sink) and
       not sink.getTypeBound() instanceof PrimitiveType and
       not sink.getTypeBound() instanceof BoxedType and
       not sink.getTypeBound() instanceof NumberType
     |
-      f instanceof ArrayContent or
-      f instanceof CollectionContent or
-      f instanceof MapKeyContent or
-      f instanceof MapValueContent
+      f instanceof DataFlow::ArrayContent or
+      f instanceof DataFlow::CollectionContent or
+      f instanceof DataFlow::MapKeyContent or
+      f instanceof DataFlow::MapValueContent
     )
     or
     FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false)
@@ -100,90 +100,31 @@ private module Cached {
 
 import Cached
 
+private RefType getElementType(RefType container) {
+  result = container.(Array).getComponentType() or
+  result = container.(CollectionType).getElementType() or
+  result = container.(MapType).getValueType()
+}
+
 /**
- * These configurations add a number of configuration-dependent additional taint
- * steps to all taint configurations. For each sink or additional step provided
- * by a given configuration the types are inspected to find those implicit
- * collection or array read steps that might be required at the sink or step
- * input. The corresponding store steps are then added as additional taint steps
- * to provide backwards-compatible taint flow to such sinks and steps.
- *
- * This is a temporary measure until support is added for such sinks that
- * require implicit read steps.
+ * Holds if default `TaintTracking::Configuration`s should allow implicit reads
+ * of `c` at sinks and inputs to additional taint steps.
  */
-private module StoreTaintSteps {
-  private import semmle.code.java.dataflow.TaintTracking
-  private import semmle.code.java.dataflow.TaintTracking2
-
-  private class StoreTaintConfig extends TaintTracking::Configuration {
-    StoreTaintConfig() { this instanceof TaintTracking::Configuration or none() }
-
-    override predicate isSource(DataFlow::Node n) { none() }
-
-    override predicate isSink(DataFlow::Node n) { none() }
-
-    private predicate needsTaintStore(RefType container, Type elem, Content f) {
-      exists(DataFlow::Node arg |
-        (isSink(arg) or isAdditionalTaintStep(arg, _)) and
-        (arg.asExpr() instanceof Argument or arg instanceof ArgumentNode) and
-        arg.getType() = container
-        or
-        needsTaintStore(_, container, _)
-      |
-        container.(Array).getComponentType() = elem and
-        f instanceof ArrayContent
-        or
-        container.(CollectionType).getElementType() = elem and
-        f instanceof CollectionContent
-        or
-        container.(MapType).getValueType() = elem and
-        f instanceof MapValueContent
-      )
-    }
-
-    override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-      exists(Content f, Type elem |
-        storeStep(node1, f, node2) and
-        needsTaintStore(_, elem, f) and
-        not exists(Type srctyp | srctyp = node1.getTypeBound() | not compatibleTypes(srctyp, elem))
-      )
-    }
-  }
-
-  private class StoreTaintConfig2 extends TaintTracking2::Configuration {
-    StoreTaintConfig2() { this instanceof TaintTracking2::Configuration or none() }
-
-    override predicate isSource(DataFlow::Node n) { none() }
-
-    override predicate isSink(DataFlow::Node n) { none() }
-
-    private predicate needsTaintStore(RefType container, Type elem, Content f) {
-      exists(DataFlow::Node arg |
-        (isSink(arg) or isAdditionalTaintStep(arg, _)) and
-        (arg.asExpr() instanceof Argument or arg instanceof ArgumentNode) and
-        arg.getType() = container
-        or
-        needsTaintStore(_, container, _)
-      |
-        container.(Array).getComponentType() = elem and
-        f instanceof ArrayContent
-        or
-        container.(CollectionType).getElementType() = elem and
-        f instanceof CollectionContent
-        or
-        container.(MapType).getValueType() = elem and
-        f instanceof MapValueContent
-      )
-    }
-
-    override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-      exists(Content f, Type elem |
-        storeStep(node1, f, node2) and
-        needsTaintStore(_, elem, f) and
-        not exists(Type srctyp | srctyp = node1.getTypeBound() | not compatibleTypes(srctyp, elem))
-      )
-    }
-  }
+bindingset[node]
+predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::Content c) {
+  exists(RefType container |
+    (node.asExpr() instanceof Argument or node instanceof ArgumentNode) and
+    getElementType*(node.getType()) = container
+  |
+    container instanceof Array and
+    c instanceof DataFlow::ArrayContent
+    or
+    container instanceof CollectionType and
+    c instanceof DataFlow::CollectionContent
+    or
+    container instanceof MapType and
+    c instanceof DataFlow::MapValueContent
+  )
 }
 
 /**

java/ql/src/semmle/code/java/dataflow/internal/tainttracking1/TaintTrackingImpl.qll

@@ -105,6 +105,11 @@ abstract class Configuration extends DataFlow::Configuration {
     defaultAdditionalTaintStep(node1, node2)
   }
 
+  override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
+    (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
+    defaultImplicitTaintRead(node, c)
+  }
+
   /**
    * Holds if taint may flow from `source` to `sink` for this configuration.
    */

java/ql/src/semmle/code/java/dataflow/internal/tainttracking2/TaintTrackingImpl.qll

@@ -105,6 +105,11 @@ abstract class Configuration extends DataFlow::Configuration {
     defaultAdditionalTaintStep(node1, node2)
   }
 
+  override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) {
+    (this.isSink(node) or this.isAdditionalTaintStep(node, _)) and
+    defaultImplicitTaintRead(node, c)
+  }
+
   /**
    * Holds if taint may flow from `source` to `sink` for this configuration.
    */

java/ql/src/semmle/code/java/frameworks/JaxWS.qll

@@ -604,13 +604,13 @@ private class FormModel extends SummaryModelCsv {
     row =
       [
         "javax.ws.rs.core;Form;false;Form;;;MapKey of Argument[0];Argument[-1];taint",
-        "javax.ws.rs.core;Form;false;Form;;;MapValue of Argument[0];Argument[-1];taint",
+        "javax.ws.rs.core;Form;false;Form;;;Element of MapValue of Argument[0];Argument[-1];taint",
         "javax.ws.rs.core;Form;false;Form;;;Argument[0..1];Argument[-1];taint",
         "javax.ws.rs.core;Form;true;asMap;;;Argument[-1];ReturnValue;taint",
         "javax.ws.rs.core;Form;true;param;;;Argument[0..1];Argument[-1];taint",
         "javax.ws.rs.core;Form;true;param;;;Argument[-1];ReturnValue;value",
         "jakarta.ws.rs.core;Form;false;Form;;;MapKey of Argument[0];Argument[-1];taint",
-        "jakarta.ws.rs.core;Form;false;Form;;;MapValue of Argument[0];Argument[-1];taint",
+        "jakarta.ws.rs.core;Form;false;Form;;;Element of MapValue of Argument[0];Argument[-1];taint",
         "jakarta.ws.rs.core;Form;false;Form;;;Argument[0..1];Argument[-1];taint",
         "jakarta.ws.rs.core;Form;true;asMap;;;Argument[-1];ReturnValue;taint",
         "jakarta.ws.rs.core;Form;true;param;;;Argument[0..1];Argument[-1];taint",

java/ql/src/semmle/code/java/frameworks/apache/Lang.qll

@@ -34,38 +34,41 @@ private class ApacheArrayUtilsModel extends SummaryModelCsv {
   override predicate row(string row) {
     row =
       [
-        "org.apache.commons.lang3;ArrayUtils;false;add;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;;;Argument[2];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(java.lang.Object[],java.lang.Object);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(boolean[],boolean);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(byte[],byte);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(char[],char);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(double[],double);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(float[],float);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(int[],int);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(long[],long);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;add;(short[],short);;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;addAll;;;Argument[0..1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;addFirst;;;Argument[0..1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;clone;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;get;(java.lang.Object[],int,java.lang.Object);;Argument[2];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;get;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;insert;;;Argument[1];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;insert;;;Argument[2];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;insert;;;Argument[3];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;nullToEmpty;(java.lang.Object[],java.lang.Class);;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;nullToEmpty;(java.lang.String[]);;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;remove;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;removeAll;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;removeAllOccurences;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;removeAllOccurrences;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;removeElement;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;removeElements;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;subarray;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;toArray;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;toMap;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;toObject;;;Argument[0];ReturnValue;taint",
-        "org.apache.commons.lang3;ArrayUtils;false;toPrimitive;;;Argument[0..1];ReturnValue;taint"
+        "org.apache.commons.lang3;ArrayUtils;false;add;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;;;Argument[2];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(java.lang.Object[],java.lang.Object);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(boolean[],boolean);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(byte[],byte);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(char[],char);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(double[],double);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(float[],float);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(int[],int);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(long[],long);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;add;(short[],short);;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;addAll;;;ArrayElement of Argument[0..1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;addFirst;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;addFirst;;;Argument[1];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;clone;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;get;(java.lang.Object[],int,java.lang.Object);;Argument[2];ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;get;;;ArrayElement of Argument[0];ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;insert;;;ArrayElement of Argument[1..2];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;nullToEmpty;(java.lang.Object[],java.lang.Class);;Argument[0];ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;nullToEmpty;(java.lang.String[]);;Argument[0];ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;remove;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;removeAll;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;removeAllOccurences;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;removeAllOccurrences;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;removeElement;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;removeElements;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;subarray;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;toArray;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;toMap;;;MapKey of ArrayElement of Argument[0];MapKey of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;toMap;;;MapValue of ArrayElement of Argument[0];MapValue of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;toMap;;;ArrayElement of ArrayElement of Argument[0];MapKey of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;toMap;;;ArrayElement of ArrayElement of Argument[0];MapValue of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;toObject;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;toPrimitive;;;ArrayElement of Argument[0];ArrayElement of ReturnValue;value",
+        "org.apache.commons.lang3;ArrayUtils;false;toPrimitive;;;Argument[1];ArrayElement of ReturnValue;value"
       ]
   }
 }