From 9cf6601d02adf1e3c9567a07164112e8ddbee881 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Fri, 3 Jul 2020 20:27:50 +0200 Subject: [PATCH 1/2] Java: Data flow for `java.util.Objects` --- .../java/dataflow/internal/DataFlowUtil.qll | 13 ++++++++++++ .../dataflow/local-flow/ObjectsTest.java | 15 +++++++++++++ .../dataflow/local-flow/flow.expected | 7 +++++++ .../library-tests/dataflow/local-flow/flow.ql | 21 +++++++++++++++++++ .../library-tests/dataflow/local-flow/options | 1 + 5 files changed, 57 insertions(+) create mode 100644 java/ql/test/library-tests/dataflow/local-flow/ObjectsTest.java create mode 100644 java/ql/test/library-tests/dataflow/local-flow/flow.expected create mode 100644 java/ql/test/library-tests/dataflow/local-flow/flow.ql create mode 100644 java/ql/test/library-tests/dataflow/local-flow/options diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll index 0abf432e1ba..8434d74d839 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -400,6 +400,19 @@ predicate simpleLocalFlowStep(Node node1, Node node2) { node2.asExpr().(ChooseExpr).getAResultExpr() = node1.asExpr() or node2.asExpr().(AssignExpr).getSource() = node1.asExpr() + or + exists(MethodAccess ma, Method m | + ma = node2.asExpr() and + m = ma.getMethod() and + m.getDeclaringType().hasQualifiedName("java.util", "Objects") and + ( + m.hasName(["requireNonNull", "requireNonNullElseGet"]) and node1.asExpr() = ma.getArgument(0) + or + m.hasName("requireNonNullElse") and node1.asExpr() = ma.getAnArgument() + or + m.hasName("toString") and node1.asExpr() = ma.getArgument(1) + ) + ) } /** diff --git a/java/ql/test/library-tests/dataflow/local-flow/ObjectsTest.java b/java/ql/test/library-tests/dataflow/local-flow/ObjectsTest.java new file mode 100644 index 00000000000..f9afcc924cd --- /dev/null +++ b/java/ql/test/library-tests/dataflow/local-flow/ObjectsTest.java @@ -0,0 +1,15 @@ +import java.util.Objects; + +class ObjectsTest { + public static void taintSteps() { + sink(Objects.requireNonNull(source())); + sink(Objects.requireNonNull(source(), "message")); + sink(Objects.requireNonNull(source(), () -> "value1")); + sink(Objects.requireNonNullElse(source(), source())); + sink(Objects.requireNonNullElseGet(source(), () -> "value2")); + sink(Objects.toString(null, source())); + } + private static T source() { return null; } + private static void sink(Object o) {} +} + diff --git a/java/ql/test/library-tests/dataflow/local-flow/flow.expected b/java/ql/test/library-tests/dataflow/local-flow/flow.expected new file mode 100644 index 00000000000..22a78a4694e --- /dev/null +++ b/java/ql/test/library-tests/dataflow/local-flow/flow.expected @@ -0,0 +1,7 @@ +| ObjectsTest.java:5:31:5:38 | source(...) | ObjectsTest.java:5:8:5:39 | requireNonNull(...) | +| ObjectsTest.java:6:31:6:38 | source(...) | ObjectsTest.java:6:8:6:50 | requireNonNull(...) | +| ObjectsTest.java:7:31:7:38 | source(...) | ObjectsTest.java:7:8:7:55 | requireNonNull(...) | +| ObjectsTest.java:8:35:8:42 | source(...) | ObjectsTest.java:8:8:8:53 | requireNonNullElse(...) | +| ObjectsTest.java:8:45:8:52 | source(...) | ObjectsTest.java:8:8:8:53 | requireNonNullElse(...) | +| ObjectsTest.java:9:38:9:45 | source(...) | ObjectsTest.java:9:8:9:62 | requireNonNullElseGet(...) | +| ObjectsTest.java:10:31:10:38 | source(...) | ObjectsTest.java:10:8:10:39 | toString(...) | diff --git a/java/ql/test/library-tests/dataflow/local-flow/flow.ql b/java/ql/test/library-tests/dataflow/local-flow/flow.ql new file mode 100644 index 00000000000..931434cbbdb --- /dev/null +++ b/java/ql/test/library-tests/dataflow/local-flow/flow.ql @@ -0,0 +1,21 @@ +import java +import semmle.code.java.dataflow.TaintTracking + +class Conf extends TaintTracking::Configuration { + Conf() { this = "conf" } + + override predicate isSource(DataFlow::Node src) { + src.asExpr().(MethodAccess).getMethod().hasName("source") + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + sink.asExpr() = ma.getAnArgument() and + ma.getMethod().hasName("sink") + ) + } +} + +from Conf c, DataFlow::Node src, DataFlow::Node sink +where c.hasFlow(src, sink) +select src, sink diff --git a/java/ql/test/library-tests/dataflow/local-flow/options b/java/ql/test/library-tests/dataflow/local-flow/options new file mode 100644 index 00000000000..3f12170222c --- /dev/null +++ b/java/ql/test/library-tests/dataflow/local-flow/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -source 14 -target 14 From 72a24972e7ad01d7a13fdb1c3754b2177ce96a0c Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Wed, 8 Jul 2020 13:30:24 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Anders Schack-Mulligen --- .../test/library-tests/dataflow/local-flow/ObjectsTest.java | 3 +-- java/ql/test/library-tests/dataflow/local-flow/flow.ql | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/java/ql/test/library-tests/dataflow/local-flow/ObjectsTest.java b/java/ql/test/library-tests/dataflow/local-flow/ObjectsTest.java index f9afcc924cd..c74dc0f434e 100644 --- a/java/ql/test/library-tests/dataflow/local-flow/ObjectsTest.java +++ b/java/ql/test/library-tests/dataflow/local-flow/ObjectsTest.java @@ -1,7 +1,7 @@ import java.util.Objects; class ObjectsTest { - public static void taintSteps() { + public static void valueSteps() { sink(Objects.requireNonNull(source())); sink(Objects.requireNonNull(source(), "message")); sink(Objects.requireNonNull(source(), () -> "value1")); @@ -12,4 +12,3 @@ class ObjectsTest { private static T source() { return null; } private static void sink(Object o) {} } - diff --git a/java/ql/test/library-tests/dataflow/local-flow/flow.ql b/java/ql/test/library-tests/dataflow/local-flow/flow.ql index 931434cbbdb..b568a1be73d 100644 --- a/java/ql/test/library-tests/dataflow/local-flow/flow.ql +++ b/java/ql/test/library-tests/dataflow/local-flow/flow.ql @@ -1,7 +1,7 @@ import java -import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.DataFlow -class Conf extends TaintTracking::Configuration { +class Conf extends DataFlow::Configuration { Conf() { this = "conf" } override predicate isSource(DataFlow::Node src) {