diff --git a/csharp/ql/test/library-tests/dataflow/reverse-flow/ReverseFlow.cs b/csharp/ql/test/library-tests/dataflow/reverse-flow/ReverseFlow.cs index 60a4fa8ea4b6..c131f58dc79e 100644 --- a/csharp/ql/test/library-tests/dataflow/reverse-flow/ReverseFlow.cs +++ b/csharp/ql/test/library-tests/dataflow/reverse-flow/ReverseFlow.cs @@ -43,7 +43,7 @@ public void M7() { var a = new A(); M8(a); - Sink(a.Field); // $ hasValueFlow=3 + Sink(a.Field); // $ MISSING: hasValueFlow=3 } public void M8(A a) diff --git a/csharp/ql/test/library-tests/dataflow/reverse-flow/ReverseFlow.expected b/csharp/ql/test/library-tests/dataflow/reverse-flow/ReverseFlow.expected index d899ddc7cb17..8dc3812f88a4 100644 --- a/csharp/ql/test/library-tests/dataflow/reverse-flow/ReverseFlow.expected +++ b/csharp/ql/test/library-tests/dataflow/reverse-flow/ReverseFlow.expected @@ -44,26 +44,6 @@ edges | ReverseFlow.cs:39:9:39:12 | [post] this access : A [field Field] : String | ReverseFlow.cs:37:17:37:18 | this [Reverse] : A [field Field] : String | provenance | | | ReverseFlow.cs:39:22:39:38 | call to method Source : String | ReverseFlow.cs:39:9:39:12 | [post] this access : A [field Field] : String | provenance | | | ReverseFlow.cs:39:22:39:38 | call to method Source : String | ReverseFlow.cs:39:9:39:12 | [post] this access : A [field Field] : String | provenance | | -| ReverseFlow.cs:45:12:45:12 | [post] access to local variable a : A [field Field] : String | ReverseFlow.cs:46:14:46:14 | access to local variable a : A [field Field] : String | provenance | | -| ReverseFlow.cs:45:12:45:12 | [post] access to local variable a : A [field Field] : String | ReverseFlow.cs:46:14:46:14 | access to local variable a : A [field Field] : String | provenance | | -| ReverseFlow.cs:46:14:46:14 | access to local variable a : A [field Field] : String | ReverseFlow.cs:46:14:46:20 | access to field Field | provenance | | -| ReverseFlow.cs:46:14:46:14 | access to local variable a : A [field Field] : String | ReverseFlow.cs:46:14:46:20 | access to field Field | provenance | | -| ReverseFlow.cs:49:22:49:22 | a [Reverse] : A [field Field] : String | ReverseFlow.cs:45:12:45:12 | [post] access to local variable a : A [field Field] : String | provenance | | -| ReverseFlow.cs:49:22:49:22 | a [Reverse] : A [field Field] : String | ReverseFlow.cs:45:12:45:12 | [post] access to local variable a : A [field Field] : String | provenance | | -| ReverseFlow.cs:52:9:52:9 | [post] access to local variable b [Reverse] : A [field Nested, field Field] : String | ReverseFlow.cs:52:20:52:20 | access to parameter a [Reverse] : A [field Field] : String | provenance | | -| ReverseFlow.cs:52:9:52:9 | [post] access to local variable b [Reverse] : A [field Nested, field Field] : String | ReverseFlow.cs:52:20:52:20 | access to parameter a [Reverse] : A [field Field] : String | provenance | | -| ReverseFlow.cs:52:20:52:20 | access to parameter a [Reverse] : A [field Field] : String | ReverseFlow.cs:49:22:49:22 | a [Reverse] : A [field Field] : String | provenance | | -| ReverseFlow.cs:52:20:52:20 | access to parameter a [Reverse] : A [field Field] : String | ReverseFlow.cs:49:22:49:22 | a [Reverse] : A [field Field] : String | provenance | | -| ReverseFlow.cs:53:12:53:12 | [post] access to local variable b : A [field Nested, field Field] : String | ReverseFlow.cs:52:9:52:9 | [post] access to local variable b [Reverse] : A [field Nested, field Field] : String | provenance | | -| ReverseFlow.cs:53:12:53:12 | [post] access to local variable b : A [field Nested, field Field] : String | ReverseFlow.cs:52:9:52:9 | [post] access to local variable b [Reverse] : A [field Nested, field Field] : String | provenance | | -| ReverseFlow.cs:56:22:56:22 | a [Reverse] : A [field Nested, field Field] : String | ReverseFlow.cs:53:12:53:12 | [post] access to local variable b : A [field Nested, field Field] : String | provenance | | -| ReverseFlow.cs:56:22:56:22 | a [Reverse] : A [field Nested, field Field] : String | ReverseFlow.cs:53:12:53:12 | [post] access to local variable b : A [field Nested, field Field] : String | provenance | | -| ReverseFlow.cs:58:9:58:9 | [post] access to parameter a : A [field Nested, field Field] : String | ReverseFlow.cs:56:22:56:22 | a [Reverse] : A [field Nested, field Field] : String | provenance | | -| ReverseFlow.cs:58:9:58:9 | [post] access to parameter a : A [field Nested, field Field] : String | ReverseFlow.cs:56:22:56:22 | a [Reverse] : A [field Nested, field Field] : String | provenance | | -| ReverseFlow.cs:58:9:58:16 | [post] access to field Nested : A [field Field] : String | ReverseFlow.cs:58:9:58:9 | [post] access to parameter a : A [field Nested, field Field] : String | provenance | | -| ReverseFlow.cs:58:9:58:16 | [post] access to field Nested : A [field Field] : String | ReverseFlow.cs:58:9:58:9 | [post] access to parameter a : A [field Nested, field Field] : String | provenance | | -| ReverseFlow.cs:58:26:58:42 | call to method Source : String | ReverseFlow.cs:58:9:58:16 | [post] access to field Nested : A [field Field] : String | provenance | | -| ReverseFlow.cs:58:26:58:42 | call to method Source : String | ReverseFlow.cs:58:9:58:16 | [post] access to field Nested : A [field Field] : String | provenance | | | ReverseFlow.cs:66:9:66:26 | [post] call to method GetNestedNested : A [field Field] : String | ReverseFlow.cs:66:9:66:26 | call to method GetNestedNested [Reverse] : A [field Field] : String | provenance | | | ReverseFlow.cs:66:9:66:26 | [post] call to method GetNestedNested : A [field Field] : String | ReverseFlow.cs:66:9:66:26 | call to method GetNestedNested [Reverse] : A [field Field] : String | provenance | | | ReverseFlow.cs:66:9:66:26 | call to method GetNestedNested [Reverse] : A [field Field] : String | ReverseFlow.cs:66:25:66:25 | [post] access to local variable a : A [field Nested, field Nested, field Field] : String | provenance | | @@ -141,28 +121,6 @@ nodes | ReverseFlow.cs:39:9:39:12 | [post] this access : A [field Field] : String | semmle.label | [post] this access : A [field Field] : String | | ReverseFlow.cs:39:22:39:38 | call to method Source : String | semmle.label | call to method Source : String | | ReverseFlow.cs:39:22:39:38 | call to method Source : String | semmle.label | call to method Source : String | -| ReverseFlow.cs:45:12:45:12 | [post] access to local variable a : A [field Field] : String | semmle.label | [post] access to local variable a : A [field Field] : String | -| ReverseFlow.cs:45:12:45:12 | [post] access to local variable a : A [field Field] : String | semmle.label | [post] access to local variable a : A [field Field] : String | -| ReverseFlow.cs:46:14:46:14 | access to local variable a : A [field Field] : String | semmle.label | access to local variable a : A [field Field] : String | -| ReverseFlow.cs:46:14:46:14 | access to local variable a : A [field Field] : String | semmle.label | access to local variable a : A [field Field] : String | -| ReverseFlow.cs:46:14:46:20 | access to field Field | semmle.label | access to field Field | -| ReverseFlow.cs:46:14:46:20 | access to field Field | semmle.label | access to field Field | -| ReverseFlow.cs:49:22:49:22 | a [Reverse] : A [field Field] : String | semmle.label | a [Reverse] : A [field Field] : String | -| ReverseFlow.cs:49:22:49:22 | a [Reverse] : A [field Field] : String | semmle.label | a [Reverse] : A [field Field] : String | -| ReverseFlow.cs:52:9:52:9 | [post] access to local variable b [Reverse] : A [field Nested, field Field] : String | semmle.label | [post] access to local variable b [Reverse] : A [field Nested, field Field] : String | -| ReverseFlow.cs:52:9:52:9 | [post] access to local variable b [Reverse] : A [field Nested, field Field] : String | semmle.label | [post] access to local variable b [Reverse] : A [field Nested, field Field] : String | -| ReverseFlow.cs:52:20:52:20 | access to parameter a [Reverse] : A [field Field] : String | semmle.label | access to parameter a [Reverse] : A [field Field] : String | -| ReverseFlow.cs:52:20:52:20 | access to parameter a [Reverse] : A [field Field] : String | semmle.label | access to parameter a [Reverse] : A [field Field] : String | -| ReverseFlow.cs:53:12:53:12 | [post] access to local variable b : A [field Nested, field Field] : String | semmle.label | [post] access to local variable b : A [field Nested, field Field] : String | -| ReverseFlow.cs:53:12:53:12 | [post] access to local variable b : A [field Nested, field Field] : String | semmle.label | [post] access to local variable b : A [field Nested, field Field] : String | -| ReverseFlow.cs:56:22:56:22 | a [Reverse] : A [field Nested, field Field] : String | semmle.label | a [Reverse] : A [field Nested, field Field] : String | -| ReverseFlow.cs:56:22:56:22 | a [Reverse] : A [field Nested, field Field] : String | semmle.label | a [Reverse] : A [field Nested, field Field] : String | -| ReverseFlow.cs:58:9:58:9 | [post] access to parameter a : A [field Nested, field Field] : String | semmle.label | [post] access to parameter a : A [field Nested, field Field] : String | -| ReverseFlow.cs:58:9:58:9 | [post] access to parameter a : A [field Nested, field Field] : String | semmle.label | [post] access to parameter a : A [field Nested, field Field] : String | -| ReverseFlow.cs:58:9:58:16 | [post] access to field Nested : A [field Field] : String | semmle.label | [post] access to field Nested : A [field Field] : String | -| ReverseFlow.cs:58:9:58:16 | [post] access to field Nested : A [field Field] : String | semmle.label | [post] access to field Nested : A [field Field] : String | -| ReverseFlow.cs:58:26:58:42 | call to method Source : String | semmle.label | call to method Source : String | -| ReverseFlow.cs:58:26:58:42 | call to method Source : String | semmle.label | call to method Source : String | | ReverseFlow.cs:66:9:66:26 | [post] call to method GetNestedNested : A [field Field] : String | semmle.label | [post] call to method GetNestedNested : A [field Field] : String | | ReverseFlow.cs:66:9:66:26 | [post] call to method GetNestedNested : A [field Field] : String | semmle.label | [post] call to method GetNestedNested : A [field Field] : String | | ReverseFlow.cs:66:9:66:26 | call to method GetNestedNested [Reverse] : A [field Field] : String | semmle.label | call to method GetNestedNested [Reverse] : A [field Field] : String | @@ -204,8 +162,6 @@ testFailures | ReverseFlow.cs:11:14:11:27 | access to field Field | ReverseFlow.cs:22:19:22:35 | call to method Source : String | ReverseFlow.cs:11:14:11:27 | access to field Field | $@ | ReverseFlow.cs:22:19:22:35 | call to method Source : String | call to method Source : String | | ReverseFlow.cs:28:14:28:30 | access to field Field | ReverseFlow.cs:39:22:39:38 | call to method Source : String | ReverseFlow.cs:28:14:28:30 | access to field Field | $@ | ReverseFlow.cs:39:22:39:38 | call to method Source : String | call to method Source : String | | ReverseFlow.cs:28:14:28:30 | access to field Field | ReverseFlow.cs:39:22:39:38 | call to method Source : String | ReverseFlow.cs:28:14:28:30 | access to field Field | $@ | ReverseFlow.cs:39:22:39:38 | call to method Source : String | call to method Source : String | -| ReverseFlow.cs:46:14:46:20 | access to field Field | ReverseFlow.cs:58:26:58:42 | call to method Source : String | ReverseFlow.cs:46:14:46:20 | access to field Field | $@ | ReverseFlow.cs:58:26:58:42 | call to method Source : String | call to method Source : String | -| ReverseFlow.cs:46:14:46:20 | access to field Field | ReverseFlow.cs:58:26:58:42 | call to method Source : String | ReverseFlow.cs:46:14:46:20 | access to field Field | $@ | ReverseFlow.cs:58:26:58:42 | call to method Source : String | call to method Source : String | | ReverseFlow.cs:67:14:67:34 | access to field Field | ReverseFlow.cs:66:36:66:52 | call to method Source : String | ReverseFlow.cs:67:14:67:34 | access to field Field | $@ | ReverseFlow.cs:66:36:66:52 | call to method Source : String | call to method Source : String | | ReverseFlow.cs:67:14:67:34 | access to field Field | ReverseFlow.cs:66:36:66:52 | call to method Source : String | ReverseFlow.cs:67:14:67:34 | access to field Field | $@ | ReverseFlow.cs:66:36:66:52 | call to method Source : String | call to method Source : String | | ReverseFlow.cs:81:14:81:20 | access to field Field | ReverseFlow.cs:80:19:80:35 | call to method Source : String | ReverseFlow.cs:81:14:81:20 | access to field Field | $@ | ReverseFlow.cs:80:19:80:35 | call to method Source : String | call to method Source : String | diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll index a169df2249cf..c6e0bcca2035 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll @@ -881,16 +881,12 @@ module MakeImplCommon Lang> { exists(Node n | this.isImplicitReadNode(n) | result = n + " [Ext]") or result = this.asNodeReverse(_) + " [Reverse]" - or - result = this.asNodeReverseReturnPosition() + " [ReverseReturn]" } Node asNode() { this = TNodeNormal(result) } Node asNodeReverse(boolean allowFwdFlowOut) { this = TNodeReverse(result, allowFwdFlowOut) } - ReturnPosition asNodeReverseReturnPosition() { this = TNodeReverseReturn(result) } - /** Gets the corresponding Node if this is a normal node or its post-implicit read node. */ Node asNodeOrImplicitRead() { this = TNodeNormal(result) or this = TNodeImplicitRead(result) } @@ -905,8 +901,6 @@ module MakeImplCommon Lang> { pragma[nomagic] private DataFlowCallable getEnclosingCallable0() { nodeEnclosingCallable(this.projectToNode(), result) - or - result = this.asNodeReverseReturnPosition().getCallable() } pragma[inline] @@ -921,8 +915,6 @@ module MakeImplCommon Lang> { nodeDataFlowType(this.asNodeReverse(_), result) or isTopType(result) and this.isImplicitReadNode(_) - or - result = this.(ReverseFlow::ReverseParamNodeEx).getAReturnNode().getDataFlowType0() } pragma[inline] @@ -1055,21 +1047,6 @@ module MakeImplCommon Lang> { private module ReverseFlow { module Cand { - // todo: check types - /** - * Holds if `p` can flow to `node` in the same callable using only - * value-preserving steps. - * - * `read` indicates whether it is contents of `p` that can flow to `node`. - */ - pragma[nomagic] - private predicate parameterValueFlowCand(ParamNode p, DataFlowType t, Node node) { - parameterValueFlowCand0(p, t, node) and - if node instanceof CastingNode - then compatibleTypesFilter(t, getNodeDataFlowType(node)) - else any() - } - /** * Holds if `p` can flow to `node` in the same callable using only * value-preserving steps. @@ -1077,49 +1054,44 @@ module MakeImplCommon Lang> { * `read` indicates whether it is contents of `p` that can flow to `node`. */ pragma[nomagic] - private predicate parameterValueFlowCand0(ParamNode p, DataFlowType t, Node node) { - p = node and - t = getNodeDataFlowType(node) + private predicate parameterValueFlowCand(ParamNode p, Node node) { + p = node or // local flow exists(Node mid | - parameterValueFlowCand(p, t, mid) and + parameterValueFlowCand(p, mid) and simpleLocalFlowStep(mid, node, _) and validParameterAliasStep(mid, node) ) or // store - exists(Node mid, DataFlowType t0 | - parameterValueFlowCand(p, t0, mid) and - Lang::storeStep(mid, _, node) and - t = getNodeDataFlowType(node) and - compatibleTypes(t0, pragma[only_bind_out](getNodeDataFlowType(mid))) + none() and + exists(Node mid | + parameterValueFlowCand(p, mid) and + Lang::storeStep(mid, _, node) ) or // read - exists(Node mid, DataFlowType t0 | - parameterValueFlowCand(p, t0, mid) and - Lang::readStep(mid, _, node) and - t = getNodeDataFlowType(node) and - compatibleTypes(t0, pragma[only_bind_out](getNodeDataFlowType(mid))) + exists(Node mid | + parameterValueFlowCand(p, mid) and + Lang::readStep(mid, _, node) ) or // flow through - exists(ParamNode p0, DataFlowType t0, ArgNode arg | - parameterValueFlowArgCand(p, t0, arg) and - argumentValueFlowsThroughCand(arg, p0, t, node) and - compatibleTypes(t0, getNodeDataFlowType(p0)) + exists(ArgNode arg | + parameterValueFlowArgCand(p, arg) and + argumentValueFlowsThroughCand(arg, node) ) } pragma[nomagic] - private predicate parameterValueFlowArgCand(ParamNode p, DataFlowType t, ArgNode arg) { - parameterValueFlowCand(p, t, arg) + private predicate parameterValueFlowArgCand(ParamNode p, ArgNode arg) { + parameterValueFlowCand(p, arg) } pragma[nomagic] predicate parameterValueFlowsToPreUpdateCand(ParamNode p, PostUpdateNode n) { - parameterValueFlowCand(p, _, n.getPreUpdateNode()) + parameterValueFlowCand(p, n.getPreUpdateNode()) } /** @@ -1130,19 +1102,21 @@ module MakeImplCommon Lang> { * `read` indicates whether it is contents of `p` that can flow to the return * node. */ - predicate parameterValueFlowReturnCand(ParamNode p, DataFlowType t, ReturnKind kind) { + predicate parameterValueFlowReturnCand(ParamNode p, ReturnKind kind) { exists(ReturnNode ret | - parameterValueFlowCand(p, t, ret) and + parameterValueFlowCand(p, ret) and kind = ret.getKind() ) } pragma[nomagic] private predicate argumentValueFlowsThroughCand0( - DataFlowCall call, ParamNode param, ArgNode arg, DataFlowType t, ReturnKind kind + DataFlowCall call, ArgNode arg, ReturnKind kind ) { - viableParamArg(call, param, arg) and - parameterValueFlowReturnCand(param, t, kind) + exists(ParamNode param | + viableParamArg(call, param, arg) and + parameterValueFlowReturnCand(param, kind) + ) } /** @@ -1151,17 +1125,17 @@ module MakeImplCommon Lang> { * * `read` indicates whether it is contents of `arg` that can flow to `out`. */ - predicate argumentValueFlowsThroughCand(ArgNode arg, ParamNode p, DataFlowType t, Node out) { + predicate argumentValueFlowsThroughCand(ArgNode arg, Node out) { exists(DataFlowCall call, ReturnKind kind | - argumentValueFlowsThroughCand0(call, arg, p, t, kind) and + argumentValueFlowsThroughCand0(call, arg, kind) and out = getAnOutNode(call, kind) ) } predicate cand(ParamNode p, Node n) { - parameterValueFlowCand(p, _, n) and + parameterValueFlowCand(p, n) and ( - parameterValueFlowReturnCand(p, _, _) + parameterValueFlowReturnCand(p, _) or parameterValueFlowsToPreUpdateCand(p, _) ) @@ -1204,9 +1178,6 @@ module MakeImplCommon Lang> { node2.asNode().(PostUpdateNode).getPreUpdateNode() = n2 ) or - node2 = node1.(ReverseParamNodeEx).getAReturnNode() and - model = "" - or node1.asNode().(PostUpdateNode).getPreUpdateNode() = node2.asNodeReverse(true) and model = "" } @@ -1227,6 +1198,7 @@ module MakeImplCommon Lang> { } predicate readStep(NodeEx node1, ContentSet c, NodeEx node2) { + none() and exists(boolean allowFwdFlowOut | Lang::storeStep(pragma[only_bind_into](node2.asNodeReverse(allowFwdFlowOut)), c, pragma[only_bind_into](node1.asNodeReverse(allowFwdFlowOut))) @@ -1269,23 +1241,18 @@ module MakeImplCommon Lang> { } } - final class ReverseParamNodeEx extends ParamNodeEx, TNodeReverseReturn { + final class ReverseParamNodeEx extends ParamNodeEx, TNodeReverse { private DataFlowCallable c_; private ParameterPositionEx pos_; ReverseParamNodeEx() { exists(ReturnPosition pos | - this = TNodeReverseReturn(pos) and - // pos = getValueReturnPosition(this.asNodeReverse(false)) and + pos = getValueReturnPosition(this.asNodeReverse(false)) and c_ = pos.getCallable() and pos_.asReturnKind() = pos.getKind() ) } - NodeEx getAReturnNode() { - this = TNodeReverseReturn(getValueReturnPosition(result.asNodeReverse(false))) - } - override predicate isParameterOf(DataFlowCallable c, ParameterPositionEx pos) { c = c_ and pos = pos_ } @@ -1383,8 +1350,6 @@ module MakeImplCommon Lang> { nodeIsHidden([n.asNode(), n.asNodeReverse(_)]) or n instanceof TNodeImplicitRead - or - n instanceof ReverseFlow::ReverseParamNodeEx } cached @@ -2227,13 +2192,6 @@ module MakeImplCommon Lang> { TNodeReverse(Node n, Boolean allowFwdFlowOut) { (ReverseFlow::Cand::cand(_, n) or n = any(PostUpdateNode p).getPreUpdateNode()) and if allowFwdFlowOut = false then ReverseFlow::Cand::cand(_, n) else any() - } or - TNodeReverseReturn(ReturnPosition pos) { - exists(ParamNode p, ReturnKind kind | - pos.getKind().(ValueReturnKind).getKind() = kind and - ReverseFlow::Cand::parameterValueFlowReturnCand(p, _, kind) and - pos.getCallable() = getNodeEnclosingCallable(p) - ) } /**