diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll index 59cc8d529a7..3da77b8ba99 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll @@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - simpleLocalFlowStep(node1, node2) and + ( + simpleLocalFlowStep(node1, node2) or + reverseStepThroughInputOutputAlias(node1, node2) + ) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll index 1d2e9052842..e8e606d9189 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll @@ -415,6 +415,22 @@ private module Cached { store(node1, tc.getContent(), node2, contentType, tc.getContainerType()) } + /** + * Holds if data can flow from `fromNode` to `toNode` because they are the post-update + * nodes of some function output and input respectively, where the output and input + * are aliases. A typical example is a function returning `this`, implementing a fluent + * interface. + */ + cached + predicate reverseStepThroughInputOutputAlias(Node fromNode, Node toNode) { + exists(Node fromPre, Node toPre | + fromPre = fromNode.(PostUpdateNode).getPreUpdateNode() and + toPre = toNode.(PostUpdateNode).getPreUpdateNode() + | + argumentValueFlowsThrough(toPre, TReadStepTypesNone(), fromPre) + ) + } + /** * Holds if the call context `call` either improves virtual dispatch in * `callable` or if it allows us to prune unreachable nodes in `callable`. diff --git a/java/ql/test/library-tests/dataflow/fluent-methods/Test.java b/java/ql/test/library-tests/dataflow/fluent-methods/Test.java new file mode 100644 index 00000000000..4c1bfad60c0 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/fluent-methods/Test.java @@ -0,0 +1,52 @@ +package smowton; + +public class Test { + + private String field; + + public Test fluentNoop() { + return this; + } + + public Test indirectlyFluentNoop() { + return this.fluentNoop(); + } + + public Test fluentSet(String x) { + this.field = x; + return this; + } + + public static Test identity(Test t) { + return t; + } + + public String get() { + return field; + } + + public static String source() { + return "taint"; + } + + public static void sink(String s) {} + + public static void test1() { + Test t = new Test(); + t.fluentNoop().fluentSet(source()).fluentNoop(); + sink(t.get()); // $hasTaintFlow=y + } + + public static void test2() { + Test t = new Test(); + Test.identity(t).fluentNoop().fluentSet(source()).fluentNoop(); + sink(t.get()); // $hasTaintFlow=y + } + + public static void test3() { + Test t = new Test(); + t.indirectlyFluentNoop().fluentSet(source()).fluentNoop(); + sink(t.get()); // $hasTaintFlow=y + } + +} diff --git a/java/ql/test/library-tests/dataflow/fluent-methods/flow.expected b/java/ql/test/library-tests/dataflow/fluent-methods/flow.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/java/ql/test/library-tests/dataflow/fluent-methods/flow.ql b/java/ql/test/library-tests/dataflow/fluent-methods/flow.ql new file mode 100644 index 00000000000..2e47268af3e --- /dev/null +++ b/java/ql/test/library-tests/dataflow/fluent-methods/flow.ql @@ -0,0 +1,30 @@ +import java +import semmle.code.java.dataflow.TaintTracking +import TestUtilities.InlineExpectationsTest + +class Conf extends TaintTracking::Configuration { + Conf() { this = "qltest:dataflow:fluent-methods" } + + override predicate isSource(DataFlow::Node n) { + n.asExpr().(MethodAccess).getMethod().hasName("source") + } + + override predicate isSink(DataFlow::Node n) { + exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument()) + } +} + +class HasFlowTest extends InlineExpectationsTest { + HasFlowTest() { this = "HasFlowTest" } + + override string getARelevantTag() { result = "hasTaintFlow" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasTaintFlow" and + exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) | + sink.getLocation() = location and + element = sink.toString() and + value = "y" + ) + } +}