From b8b38e4bbe7e9c630b74965156df3c2d3eca93c9 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 25 Jul 2023 15:37:41 +0200 Subject: [PATCH 1/3] Java: Allow flow out of FieldValueNodes for non-static fields --- .../code/java/dataflow/internal/DataFlowPrivate.qll | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll index 216523023d9..af698d10b6e 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/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 From 602eb431091f3c0e2f7c67cd4fdbf4ceab1f9438 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 26 Jul 2023 09:32:13 +0200 Subject: [PATCH 2/3] Update partial flow test expectations --- java/ql/test/library-tests/dataflow/partial/test.expected | 1 + java/ql/test/library-tests/dataflow/partial/testRev.expected | 2 ++ 2 files changed, 3 insertions(+) diff --git a/java/ql/test/library-tests/dataflow/partial/test.expected b/java/ql/test/library-tests/dataflow/partial/test.expected index 0b3cebcb18b..2e04a437d9f 100644 --- a/java/ql/test/library-tests/dataflow/partial/test.expected +++ b/java/ql/test/library-tests/dataflow/partial/test.expected @@ -1,5 +1,6 @@ edges | A.java:4:16:4:18 | 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 | diff --git a/java/ql/test/library-tests/dataflow/partial/testRev.expected b/java/ql/test/library-tests/dataflow/partial/testRev.expected index bd44f641276..7c2ad279b7b 100644 --- a/java/ql/test/library-tests/dataflow/partial/testRev.expected +++ b/java/ql/test/library-tests/dataflow/partial/testRev.expected @@ -1,5 +1,6 @@ edges | A.java:4:16:4:18 | 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 [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(...) | From 8685242c16018cad60bfa3d62e32ff6364358c14 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 26 Jul 2023 12:43:00 +0200 Subject: [PATCH 3/3] Add tests --- .../library-tests/dataflow/field-value/A.java | 25 +++++++++++++++++++ .../dataflow/field-value/test.expected | 2 ++ .../dataflow/field-value/test.ql | 10 ++++++++ 3 files changed, 37 insertions(+) create mode 100644 java/ql/test/library-tests/dataflow/field-value/A.java create mode 100644 java/ql/test/library-tests/dataflow/field-value/test.expected create mode 100644 java/ql/test/library-tests/dataflow/field-value/test.ql diff --git a/java/ql/test/library-tests/dataflow/field-value/A.java b/java/ql/test/library-tests/dataflow/field-value/A.java new file mode 100644 index 00000000000..d5e73b34634 --- /dev/null +++ b/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 + } + } +} diff --git a/java/ql/test/library-tests/dataflow/field-value/test.expected b/java/ql/test/library-tests/dataflow/field-value/test.expected new file mode 100644 index 00000000000..48de9172b36 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/field-value/test.expected @@ -0,0 +1,2 @@ +failures +testFailures diff --git a/java/ql/test/library-tests/dataflow/field-value/test.ql b/java/ql/test/library-tests/dataflow/field-value/test.ql new file mode 100644 index 00000000000..4c364e8df70 --- /dev/null +++ b/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