diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll index bd6149d4138fa..043aaf2108aab 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll @@ -109,7 +109,7 @@ module MakeImplCommon Lang> { } predicate simpleLocalSmallStep(Node node1, Node node2) { - simpleLocalFlowStepExt(node1, node2, _) + simpleLocalFlowStepCached(node1, node2, _) } predicate levelStepNoCall(Node n1, LocalSourceNode n2) { none() } @@ -878,7 +878,7 @@ module MakeImplCommon Lang> { Node asNode() { this = TNodeNormal(result) } - Node asNodeReverse(boolean b) { this = TNodeReverse(result, b) } + Node asNodeReverse(boolean allowFwdFlowOut) { this = TNodeReverse(result, allowFwdFlowOut) } /** 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) } @@ -926,18 +926,18 @@ module MakeImplCommon Lang> { } final class DataFlowCallEx extends MkDataFlowCallEx { - DataFlowCall asDataFlowCall(boolean b) { this = MkDataFlowCallEx(result, b) } + DataFlowCall asDataFlowCall(boolean allowFwdFlowOut) { + this = MkDataFlowCallEx(result, allowFwdFlowOut) + } - // DataFlowCall asDataFlowCallReverse(boolean b) { this = TReverseDataFlowCall(result, b) } - // DataFlowCall projectToCall() { result = [this.asDataFlowCall(), this.asDataFlowCallReverse(_)] } DataFlowCallable getEnclosingCallable() { result = this.asDataFlowCall(_).getEnclosingCallable() } string toString() { - exists(boolean b, string s | - s = this.asDataFlowCall(b).toString() and - if b = true then result = s else result = s + " (no flow out to post)" + exists(boolean allowFwdFlowOut, string s | + s = this.asDataFlowCall(allowFwdFlowOut).toString() and + if allowFwdFlowOut = true then result = s else result = s + " (no fwd flow out)" ) } @@ -975,9 +975,10 @@ module MakeImplCommon Lang> { ArgNodeEx() { this.asNode().(ArgNode).argumentOf(call_.asDataFlowCall(true), pos_.asArgumentPosition()) or - exists(boolean b | - pragma[only_bind_into](this.asNodeReverse(b)) = - getAnOutNode(call_.asDataFlowCall(b), pos_.asReturnKind().(ValueReturnKind).getKind()) + exists(boolean allowFwdFlowOut | + pragma[only_bind_into](this.asNodeReverse(allowFwdFlowOut)) = + getAnOutNode(call_.asDataFlowCall(allowFwdFlowOut), + pos_.asReturnKind().(ValueReturnKind).getKind()) ) } @@ -1054,9 +1055,7 @@ module MakeImplCommon Lang> { predicate forceCachingInSameStage() { any() } cached - newtype TDataFlowCallEx = - // NormalDataFlowCall(DataFlowCall call) or - MkDataFlowCallEx(DataFlowCall call, Boolean b) + newtype TDataFlowCallEx = MkDataFlowCallEx(DataFlowCall call, Boolean allowFwdFlowOut) cached newtype TArgumentPositionEx = @@ -1148,16 +1147,16 @@ module MakeImplCommon Lang> { cached OutNodeEx getAnOutNodeEx(DataFlowCallEx call, ReturnKindExt k) { - exists(DataFlowCall c, boolean b | c = call.asDataFlowCall(b) | + exists(DataFlowCall c, boolean allowFwdFlowOut | c = call.asDataFlowCall(allowFwdFlowOut) | result.asNode() = getAnOutNode(c, k.(ValueReturnKind).getKind()) or exists(ArgNode arg | arg.argumentOf(c, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition()) | - b = false and + allowFwdFlowOut = false and result.asNodeReverse(_) = arg or - b = true and + allowFwdFlowOut = true and result.asNode().(PostUpdateNode).getPreUpdateNode() = arg ) ) @@ -1778,9 +1777,9 @@ module MakeImplCommon Lang> { predicate readEx(NodeEx node1, ContentSet c, NodeEx node2) { readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode())) or - exists(boolean b | - storeSet(pragma[only_bind_into](node2.asNodeReverse(b)), c, - pragma[only_bind_into](node1.asNodeReverse(b)), _, _) + exists(boolean allowFwdFlowOut | + storeSet(pragma[only_bind_into](node2.asNodeReverse(allowFwdFlowOut)), c, + pragma[only_bind_into](node1.asNodeReverse(allowFwdFlowOut)), _, _) ) } @@ -1819,13 +1818,15 @@ module MakeImplCommon Lang> { storeSet(pragma[only_bind_into](node1.asNode()), cs, pragma[only_bind_into](node2.asNode()), contentType, containerType) or - exists(Node n1, Node n2, boolean b | - n1 = pragma[only_bind_into](node1.asNodeReverse(b)) and - n2 = pragma[only_bind_into](node2.asNodeReverse(b)) and + exists(Node n1, Node n2, boolean allowFwdFlowOut | + n1 = pragma[only_bind_into](node1.asNodeReverse(allowFwdFlowOut)) and + n2 = pragma[only_bind_into](node2.asNodeReverse(allowFwdFlowOut)) and readSet(n2, cs, n1) and - contentType = getNodeDataFlowType(node1.asNodeReverse(b)) and - containerType = getNodeDataFlowType(node2.asNodeReverse(b)) and - not exists(PostUpdateNode pn1, PostUpdateNode pn2 | + contentType = getNodeDataFlowType(n1) and + containerType = getNodeDataFlowType(n2) and + not exists( + PostUpdateNode pn1, PostUpdateNode pn2 // why? + | n1 = pn1.getPreUpdateNode() and n2 = pn2.getPreUpdateNode() ) @@ -1833,35 +1834,9 @@ module MakeImplCommon Lang> { ) } - /** - * 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. - */ - private predicate reverseStepThroughInputOutputAlias( - PostUpdateNode fromNode, PostUpdateNode toNode, string model - ) { - exists(Node fromPre, Node toPre | - fromPre = fromNode.getPreUpdateNode() and - toPre = toNode.getPreUpdateNode() - | - exists(DataFlowCall c | - // Does the language-specific simpleLocalFlowStep already model flow - // from function input to output? - fromPre = getAnOutNode(c, _) and - toPre.(ArgNode).argumentOf(c, _) and - simpleLocalFlowStep(toPre.(ArgNode), fromPre, model) - ) - or - argumentValueFlowsThrough(toPre, TReadStepTypesNone(), fromPre, model) - ) - } - cached - predicate simpleLocalFlowStepExt(Node node1, Node node2, string model) { - simpleLocalFlowStep(node1, node2, model) or - reverseStepThroughInputOutputAlias(node1, node2, model) + predicate simpleLocalFlowStepCached(Node node1, Node node2, string model) { + simpleLocalFlowStep(node1, node2, model) } cached @@ -2016,7 +1991,7 @@ module MakeImplCommon Lang> { newtype TNodeEx = TNodeNormal(Node n) or TNodeImplicitRead(Node n) or - TNodeReverse(Node n, Boolean b) + TNodeReverse(Node n, Boolean allowFwdFlowOut) /** * Holds if data can flow in one local step from `node1` to `node2`. @@ -2026,13 +2001,15 @@ module MakeImplCommon Lang> { exists(Node n1, Node n2 | node1.asNode() = n1 and node2.asNode() = n2 and - simpleLocalFlowStepExt(pragma[only_bind_into](n1), pragma[only_bind_into](n2), model) + simpleLocalFlowStep(pragma[only_bind_into](n1), pragma[only_bind_into](n2), model) ) or + // as soon as we take a reverse local step, forward out flow must be prohibited + // in order to prevent time travelling exists(Node n1, Node n2 | node1.asNodeReverse(_) = n1 and node2.asNodeReverse(false) = n2 and - simpleLocalFlowStepExt(pragma[only_bind_into](n2), pragma[only_bind_into](n1), model) + simpleLocalFlowStep(pragma[only_bind_into](n2), pragma[only_bind_into](n1), model) ) or node1.asNode().(PostUpdateNode).getPreUpdateNode() = node2.asNodeReverse(true) and