Java: Bugfix for flow through methods with taintstep and upcast.

Author: aschackmull

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

@@ -851,8 +851,6 @@ private class AccessPathFrontNilNode extends Node {
       localFlowBigStep(_, this, false, _)
       or
       additionalJumpStep(_, this, _)
-      or
-      simpleArgumentFlowsThrough(_, this, _, _)
     )
   }
 
@@ -914,10 +912,10 @@ private predicate flowCandFwd0(Node node, boolean fromArg, AccessPathFront apf,
       argumentValueFlowsThrough(mid, node, _)
     )
     or
-    exists(Node mid, AccessPathFrontNil nil |
+    exists(Node mid, AccessPathFrontNil nil, DataFlowType t |
       flowCandFwd(mid, fromArg, nil, config) and
-      simpleArgumentFlowsThrough(mid, node, _, config) and
-      apf = node.(AccessPathFrontNilNode).getApf()
+      simpleArgumentFlowsThrough(mid, node, t, config) and
+      apf = TFrontNil(t)
     )
   )
   or
@@ -1187,10 +1185,10 @@ private predicate flowFwd0(
       argumentValueFlowsThrough(mid, node, _)
     )
     or
-    exists(Node mid, AccessPathNil nil |
+    exists(Node mid, AccessPathNil nil, DataFlowType t |
       flowFwd(mid, fromArg, _, nil, config) and
-      simpleArgumentFlowsThrough(mid, node, _, config) and
-      ap = node.(AccessPathNilNode).getAp() and
+      simpleArgumentFlowsThrough(mid, node, t, config) and
+      ap = TNil(t) and
       apf = ap.(AccessPathNil).getFront()
     )
   )
@@ -1539,7 +1537,7 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, AccessPat
   or
   pathOutOfCallable(mid, node, cc) and ap = mid.getAp()
   or
-  pathThroughCallable(mid, node, cc) and ap = node.(AccessPathNilNode).getAp()
+  pathThroughCallable(mid, node, cc, ap)
   or
   valuePathThroughCallable(mid, node, cc) and ap = mid.getAp()
 }
@@ -1670,14 +1668,14 @@ private predicate pathIntoCallable(
 /** Holds if data may flow from `p` to a return of kind `kind`. */
 pragma[nomagic]
 private predicate paramFlowsThrough(
-  ParameterNode p, ReturnKind kind, CallContextCall cc, Configuration config
+  ParameterNode p, ReturnKind kind, CallContextCall cc, AccessPathNil apnil, Configuration config
 ) {
   exists(PathNodeMid mid, ReturnNode ret |
     mid.getNode() = ret and
     kind = ret.getKind() and
     cc = mid.getCallContext() and
     config = mid.getConfiguration() and
-    mid.getAp() instanceof AccessPathNil
+    apnil = mid.getAp()
   |
     cc = TSomeCall(p, true)
     or
@@ -1689,11 +1687,11 @@ private predicate paramFlowsThrough(
 
 pragma[noinline]
 private predicate pathThroughCallable0(
-  DataFlowCall call, PathNodeMid mid, ReturnKind kind, CallContext cc
+  DataFlowCall call, PathNodeMid mid, ReturnKind kind, CallContext cc, AccessPathNil apnil
 ) {
   exists(ParameterNode p, CallContext innercc |
     pathIntoCallable(mid, p, cc, innercc, call) and
-    paramFlowsThrough(p, kind, innercc, unbind(mid.getConfiguration())) and
+    paramFlowsThrough(p, kind, innercc, apnil, unbind(mid.getConfiguration())) and
     not parameterValueFlowsThrough(p, kind, innercc) and
     mid.getAp() instanceof AccessPathNil
   )
@@ -1704,9 +1702,11 @@ private predicate pathThroughCallable0(
  * The context `cc` is restored to its value prior to entering the callable.
  */
 pragma[noinline]
-private predicate pathThroughCallable(PathNodeMid mid, OutNode out, CallContext cc) {
+private predicate pathThroughCallable(
+  PathNodeMid mid, OutNode out, CallContext cc, AccessPathNil apnil
+) {
   exists(DataFlowCall call, ReturnKind kind |
-    pathThroughCallable0(call, mid, kind, cc) and
+    pathThroughCallable0(call, mid, kind, cc, apnil) and
     out = getAnOutNode(call, kind)
   )
 }

java/ql/test/library-tests/dataflow/taintreturn/Test.java

@@ -0,0 +1,22 @@
+public class Test {
+  static class A {
+    B step() { return null; }
+  }
+  static class B extends C { }
+  static class C { }
+
+  A src() { return new A(); }
+
+  void sink(Object o) { }
+
+  void flow() {
+    A a = src();
+    C c = m1(a);
+    sink(c);
+  }
+
+  C m1(A a) {
+    return a.step();
+  }
+}
+

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

@@ -0,0 +1 @@
+| 1 | 1 |

java/ql/test/library-tests/dataflow/taintreturn/taintreturn.ql

@@ -0,0 +1,44 @@
+import java
+import semmle.code.java.dataflow.TaintTracking
+import DataFlow
+
+predicate step(Expr e1, Expr e2) {
+  exists(MethodAccess ma |
+    ma.getMethod().hasName("step") and
+    ma = e2 and
+    ma.getQualifier() = e1
+  )
+}
+
+predicate isSink0(Expr sink) {
+  exists(MethodAccess ma |
+    ma.getMethod().hasName("sink") and
+    ma.getAnArgument() = sink
+  )
+}
+
+class Conf1 extends Configuration {
+  Conf1() { this = "testconf1" }
+
+  override predicate isSource(Node n) { n.asExpr().(MethodAccess).getMethod().hasName("src") }
+
+  override predicate isSink(Node n) { any() }
+
+  override predicate isAdditionalFlowStep(Node n1, Node n2) { step(n1.asExpr(), n2.asExpr()) }
+}
+
+class Conf2 extends Configuration {
+  Conf2() { this = "testconf2" }
+
+  override predicate isSource(Node n) { n.asExpr().(MethodAccess).getMethod().hasName("src") }
+
+  override predicate isSink(Node n) { isSink0(n.asExpr()) }
+
+  override predicate isAdditionalFlowStep(Node n1, Node n2) { step(n1.asExpr(), n2.asExpr()) }
+}
+
+from int i1, int i2
+where
+  i1 = count(Node src, Node sink, Conf1 c | c.hasFlow(src, sink) and isSink0(sink.asExpr())) and
+  i2 = count(Node src, Node sink, Conf2 c | c.hasFlow(src, sink))
+select i1, i2