Merge pull request #13817 from atorralba/atorralba/java/non-static-fieldvaluenode-step

Java: Allow flow out of FieldValueNodes for non-static fields

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

@@ -33,17 +33,17 @@ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
 }
 
 /**
- * Holds if data can flow from `node1` to `node2` through a static field.
+ * Holds if data can flow from `node1` to `node2` through a field.
  */
-private predicate staticFieldStep(Node node1, Node node2) {
+private predicate fieldStep(Node node1, Node node2) {
   exists(Field f |
+    // Taint fields through assigned values only if they're static
     f.isStatic() and
     f.getAnAssignedValue() = node1.asExpr() and
     node2.(FieldValueNode).getField() = f
   )
   or
   exists(Field f, FieldRead fr |
-    f.isStatic() and
     node1.(FieldValueNode).getField() = f and
     fr.getField() = f and
     fr = node2.asExpr() and
@@ -72,11 +72,11 @@ private predicate variableCaptureStep(Node node1, ExprNode node2) {
 }
 
 /**
- * Holds if data can flow from `node1` to `node2` through a static field or
+ * Holds if data can flow from `node1` to `node2` through a field or
  * variable capture.
  */
 predicate jumpStep(Node node1, Node node2) {
-  staticFieldStep(node1, node2)
+  fieldStep(node1, node2)
   or
   variableCaptureStep(node1, node2)
   or

java/ql/test/library-tests/dataflow/field-value/A.java

@@ -0,0 +1,25 @@
+import java.io.FilterInputStream;
+import java.io.InputStream;
+
+public class A {
+
+    public String src;
+
+    private static void sink(Object o) {}
+
+    public void test() {
+        sink(src); // $ hasTaintFlow
+    }
+
+    class TestFis extends FilterInputStream {
+
+        protected TestFis(InputStream in) {
+            super(in);
+        }
+
+        public void testOutOfSource() {
+            // out of source field
+            sink(this.in); // $ hasTaintFlow
+        }
+    }
+}

java/ql/test/library-tests/dataflow/field-value/test.expected

@@ -0,0 +1,2 @@
+failures
+testFailures

java/ql/test/library-tests/dataflow/field-value/test.ql

@@ -0,0 +1,10 @@
+import java
+import TestUtilities.InlineFlowTest
+
+module FieldValueConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof DataFlow::FieldValueNode }
+
+  predicate isSink(DataFlow::Node sink) { DefaultFlowConfig::isSink(sink) }
+}
+
+import TaintFlowTest<FieldValueConfig>

java/ql/test/library-tests/dataflow/partial/test.expected

@@ -1,5 +1,6 @@
 edges
 | A.java:4:16:4:18 | this <constr(this)> [post update] [elem] | A.java:22:17:22:25 | new Box(...) [elem] |
+| A.java:5:19:5:22 | elem | A.java:24:10:24:19 | other.elem |
 | A.java:12:5:12:5 | b [post update] : Box [elem] | A.java:13:12:13:12 | b : Box [elem] |
 | A.java:12:14:12:18 | src(...) : Object | A.java:12:5:12:5 | b [post update] : Box [elem] |
 | A.java:12:14:12:18 | src(...) : Object | A.java:12:5:12:18 | ...=... : Object |

java/ql/test/library-tests/dataflow/partial/testRev.expected

@@ -1,5 +1,6 @@
 edges
 | A.java:4:16:4:18 | this <constr(this)> [post update] [elem] | A.java:22:17:22:25 | new Box(...) [elem] |
+| A.java:5:19:5:22 | elem | A.java:24:10:24:19 | other.elem |
 | A.java:12:5:12:5 | b [post update] : Box [elem] | A.java:13:12:13:12 | b : Box [elem] |
 | A.java:12:14:12:18 | src(...) : Object | A.java:12:5:12:5 | b [post update] : Box [elem] |
 | A.java:12:14:12:18 | src(...) : Object | A.java:12:5:12:18 | ...=... : Object |
@@ -18,5 +19,6 @@ edges
 | 0 | A.java:23:13:23:17 | other [post update] [elem] |
 | 0 | A.java:24:10:24:14 | other [elem] |
 | 1 | A.java:4:16:4:18 | this <constr(this)> [post update] [elem] |
+| 1 | A.java:5:19:5:22 | elem |
 | 1 | A.java:28:5:28:5 | b [post update] [elem] |
 | 1 | A.java:28:14:28:25 | new Object(...) |