diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll index 0c474b0e75d0..ff5f3e46e648 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ProductFlow.qll @@ -545,7 +545,7 @@ module ProductFlow { private predicate outImpl1(Flow1::PathNode pred1, Flow1::PathNode succ1, DataFlowCall call) { Flow1::PathGraph::edges(pred1, succ1, _, _) and exists(ReturnKindExt returnKind | - succ1.getNode() = returnKind.getAnOutNode(call) and + succ1.getNode() = getAnOutNodeExt(call, returnKind) and returnKind = getParamReturnPosition(_, pred1.asParameterReturnNode()).getKind() ) } @@ -573,7 +573,7 @@ module ProductFlow { private predicate outImpl2(Flow2::PathNode pred2, Flow2::PathNode succ2, DataFlowCall call) { Flow2::PathGraph::edges(pred2, succ2, _, _) and exists(ReturnKindExt returnKind | - succ2.getNode() = returnKind.getAnOutNode(call) and + succ2.getNode() = getAnOutNodeExt(call, returnKind) and returnKind = getParamReturnPosition(_, pred2.asParameterReturnNode()).getKind() ) } diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll index d4c73c234beb..96184a607f0b 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll @@ -343,7 +343,7 @@ module MakeImpl Lang> { bindingset[n, cc] pragma[inline_late] private predicate isUnreachableInCall1(NodeEx n, LocalCallContextSpecificCall cc) { - cc.unreachable(n.asNode()) + cc.unreachable(n) } /** @@ -423,7 +423,7 @@ module MakeImpl Lang> { pragma[nomagic] private predicate readSetEx(NodeEx node1, ContentSet c, NodeEx node2) { - readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode())) and + readEx(node1, c, node2) and stepFilter(node1, node2) or exists(Node n | @@ -450,20 +450,19 @@ module MakeImpl Lang> { bindingset[c] private predicate expectsContentEx(NodeEx n, Content c) { exists(ContentSet cs | - expectsContentCached(n.asNode(), cs) and + expectsContentSet(n, cs) and pragma[only_bind_out](c) = pragma[only_bind_into](cs).getAReadContent() ) } pragma[nomagic] - private predicate notExpectsContent(NodeEx n) { not expectsContentCached(n.asNode(), _) } + private predicate notExpectsContent(NodeEx n) { not expectsContentSet(n, _) } pragma[nomagic] - private predicate storeExUnrestricted( + private predicate storeUnrestricted( NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType ) { - store(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode()), - contentType, containerType) and + storeEx(node1, c, node2, contentType, containerType) and stepFilter(node1, node2) } @@ -471,23 +470,13 @@ module MakeImpl Lang> { private predicate hasReadStep(Content c) { read(_, c, _) } pragma[nomagic] - private predicate storeEx( + private predicate store( NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType ) { - storeExUnrestricted(node1, c, node2, contentType, containerType) and + storeUnrestricted(node1, c, node2, contentType, containerType) and hasReadStep(c) } - pragma[nomagic] - private predicate viableReturnPosOutEx(DataFlowCall call, ReturnPosition pos, NodeEx out) { - viableReturnPosOut(call, pos, out.asNode()) - } - - pragma[nomagic] - private predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx arg) { - viableParamArg(call, p.asNode(), arg.asNode()) - } - /** * Holds if field flow should be used for the given configuration. */ @@ -520,7 +509,7 @@ module MakeImpl Lang> { exists(ParameterPosition pos | p.isParameterOf(_, pos) | not kind.(ParamUpdateReturnKind).getPosition() = pos or - allowParameterReturnInSelfCached(p.asNode()) + allowParameterReturnInSelfEx(p) ) } @@ -558,7 +547,7 @@ module MakeImpl Lang> { exists(NodeEx mid | useFieldFlow() and fwdFlow(mid, cc) and - storeEx(mid, _, node, _, _) + store(mid, _, node, _, _) ) or // read @@ -653,7 +642,7 @@ module MakeImpl Lang> { not fullBarrier(node) and useFieldFlow() and fwdFlow(mid, _) and - storeEx(mid, c, node, _, _) + store(mid, c, node, _, _) ) } @@ -796,7 +785,7 @@ module MakeImpl Lang> { exists(NodeEx mid | revFlow(mid, toReturn) and fwdFlowConsCand(c) and - storeEx(node, c, mid, _, _) + store(node, c, mid, _, _) ) } @@ -893,7 +882,7 @@ module MakeImpl Lang> { ) { revFlowIsReadAndStored(c) and revFlow(node2) and - storeEx(node1, c, node2, contentType, containerType) and + store(node1, c, node2, contentType, containerType) and exists(ap1) } @@ -1152,7 +1141,7 @@ module MakeImpl Lang> { flowOutOfCallNodeCand1(call, ret, _, out) and c = ret.getEnclosingCallable() | - scope = getSecondLevelScopeCached(ret.asNode()) + scope = getSecondLevelScopeEx(ret) or ret = TParamReturnNode(_, scope) ) @@ -1496,7 +1485,7 @@ module MakeImpl Lang> { PrevStage::revFlow(node, state, apa) and filter(node, state, t0, ap, t) and ( - if castingNodeEx(node) + if node instanceof CastingNodeEx then ap instanceof ApNil or compatibleContainer(getHeadContent(ap), node.getDataFlowType()) or @@ -2627,10 +2616,7 @@ module MakeImpl Lang> { FlowCheckNode() { revFlow(this, _, _) and ( - castNode(this.asNode()) or - clearsContentCached(this.asNode(), _) or - expectsContentCached(this.asNode(), _) or - neverSkipInPathGraph(this.asNode()) or + flowCheckNode(this) or Config::neverSkip(this.asNode()) ) } @@ -2665,7 +2651,7 @@ module MakeImpl Lang> { or node instanceof ParamNodeEx or - node.asNode() instanceof OutNodeExt + node instanceof OutNodeEx or storeStepCand(_, _, _, node, _, _) or @@ -2899,15 +2885,9 @@ module MakeImpl Lang> { predicate isHidden() { not Config::includeHiddenNodes() and - ( - hiddenNode(this.getNodeEx().asNode()) and - not this.isSource() and - not this instanceof PathNodeSink - or - this.getNodeEx() instanceof TNodeImplicitRead - or - hiddenNode(this.getNodeEx().asParamReturnNode()) - ) + hiddenNode(this.getNodeEx()) and + not this.isSource() and + not this instanceof PathNodeSink } /** Gets a textual representation of this element. */ @@ -3770,11 +3750,6 @@ module MakeImpl Lang> { private module Stage2 = MkStage::Stage; - pragma[nomagic] - private predicate castingNodeEx(NodeEx node) { - node.asNode() instanceof CastingNode or exists(node.asParamReturnNode()) - } - private module Stage3Param implements MkStage::StageParam { private module PrevStage = Stage2; @@ -3888,7 +3863,7 @@ module MakeImpl Lang> { bindingset[node, t0] private predicate strengthenType(NodeEx node, DataFlowType t0, DataFlowType t) { - if castingNodeEx(node) + if node instanceof CastingNodeEx then exists(DataFlowType nt | nt = node.getDataFlowType() | if typeStrongerThanFilter(nt, t0) @@ -3945,7 +3920,7 @@ module MakeImpl Lang> { pragma[nomagic] private predicate clearSet(NodeEx node, ContentSet c) { PrevStage::revFlow(node) and - clearsContentCached(node.asNode(), c) + clearsContentSet(node, c) } pragma[nomagic] @@ -5024,7 +4999,7 @@ module MakeImpl Lang> { bindingset[c] private predicate clearsContentEx(NodeEx n, Content c) { exists(ContentSet cs | - clearsContentCached(n.asNode(), cs) and + clearsContentSet(n, cs) and pragma[only_bind_out](c) = pragma[only_bind_into](cs).getAReadContent() ) } @@ -5377,7 +5352,7 @@ module MakeImpl Lang> { midNode = mid.getNodeEx() and t1 = mid.getType() and ap1 = mid.getAp() and - storeExUnrestricted(midNode, c, node, contentType, t2) and + storeUnrestricted(midNode, c, node, contentType, t2) and ap2.getHead() = c and ap2.len() = unbindInt(ap1.len() + 1) and compatibleTypesFilter(t1, contentType) @@ -5442,9 +5417,8 @@ module MakeImpl Lang> { PartialAccessPath ap ) { exists(ReturnKindExt kind, DataFlowCall call | - partialPathOutOfCallable1(mid, call, kind, state, cc, t, ap) - | - out.asNode() = kind.getAnOutNode(call) + partialPathOutOfCallable1(mid, call, kind, state, cc, t, ap) and + out = kind.getAnOutNodeEx(call) ) } @@ -5529,7 +5503,7 @@ module MakeImpl Lang> { ) { exists(DataFlowCall call, ReturnKindExt kind | partialPathThroughCallable0(call, mid, kind, state, cc, t, ap) and - out.asNode() = kind.getAnOutNode(call) + out = kind.getAnOutNodeEx(call) ) } @@ -5549,7 +5523,7 @@ module MakeImpl Lang> { not outBarrier(node, state) and // if a node is not the target of a store, we can check `clearsContent` immediately ( - storeExUnrestricted(_, _, node, _, _) + storeUnrestricted(_, _, node, _, _) or not clearsContentEx(node, ap.getHead()) ) @@ -5690,7 +5664,7 @@ module MakeImpl Lang> { exists(NodeEx midNode | midNode = mid.getNodeEx() and ap = mid.getAp() and - storeExUnrestricted(node, c, midNode, _, _) and + storeUnrestricted(node, c, midNode, _, _) and ap.getHead() = c ) } @@ -5745,7 +5719,7 @@ module MakeImpl Lang> { ) { exists(DataFlowCall call, ArgumentPosition pos | revPartialPathThroughCallable0(call, mid, pos, state, ap) and - node.asNode().(ArgNode).argumentOf(call, pos) + node.argumentOf(call, pos) ) } diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll index e477913a59bd..75d68cf247c1 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll @@ -286,7 +286,7 @@ module MakeImplCommon Lang> { ) { revLambdaFlow0(lambdaCall, kind, node, t, toReturn, toJump, lastCall) and not expectsContent(node, _) and - if castNode(node) or node instanceof ArgNode or node instanceof ReturnNode + if node instanceof CastNode or node instanceof ArgNode or node instanceof ReturnNode then compatibleTypesFilter(t, getNodeDataFlowType(node)) else any() } @@ -372,7 +372,7 @@ module MakeImplCommon Lang> { ) { revLambdaFlow(lambdaCall, kind, out, t, _, toJump, lastCall) and exists(ReturnKindExt rk | - out = rk.getAnOutNode(call) and + out = getAnOutNodeExt(call, rk) and lambdaCall(call, _, _) ) } @@ -901,20 +901,36 @@ module MakeImplCommon Lang> { Location getLocation() { result = this.projectToNode().getLocation() } } + /** + * A `Node` at which a cast can occur such that the type should be checked. + */ + final class CastingNodeEx extends NodeEx { + CastingNodeEx() { castingNodeEx(this) } + } + final class ArgNodeEx extends NodeEx { - ArgNodeEx() { this.asNode() instanceof ArgNode } + private DataFlowCall call_; + private ArgumentPosition pos_; + + ArgNodeEx() { this.asNode().(ArgNode).argumentOf(call_, pos_) } + + predicate argumentOf(DataFlowCall call, ArgumentPosition pos) { + call = call_ and + pos = pos_ + } - DataFlowCall getCall() { this.asNode().(ArgNode).argumentOf(result, _) } + DataFlowCall getCall() { result = call_ } } final class ParamNodeEx extends NodeEx { - ParamNodeEx() { this.asNode() instanceof ParamNode } + private DataFlowCallable c_; + private ParameterPosition pos_; - predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { - this.asNode().(ParamNode).isParameterOf(c, pos) - } + ParamNodeEx() { this.asNode().(ParamNode).isParameterOf(c_, pos_) } + + predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { c = c_ and pos = pos_ } - ParameterPosition getPosition() { this.isParameterOf(_, result) } + ParameterPosition getPosition() { result = pos_ } } /** @@ -931,6 +947,18 @@ module MakeImplCommon Lang> { ReturnKindExt getKind() { result = pos.getKind() } } + final class OutNodeEx extends NodeEx { + OutNodeEx() { this.asNode() instanceof OutNodeExt } + } + + pragma[nomagic] + private SndLevelScopeOption getSecondLevelScope0(Node n) { + result = SndLevelScopeOption::some(getSecondLevelScope(n)) + or + result instanceof SndLevelScopeOption::None and + not exists(getSecondLevelScope(n)) + } + cached private module Cached { /** @@ -942,11 +970,8 @@ module MakeImplCommon Lang> { predicate forceCachingInSameStage() { any() } cached - SndLevelScopeOption getSecondLevelScopeCached(Node n) { - result = SndLevelScopeOption::some(getSecondLevelScope(n)) - or - result instanceof SndLevelScopeOption::None and - not exists(getSecondLevelScope(n)) + SndLevelScopeOption getSecondLevelScopeEx(NodeEx n) { + result = getSecondLevelScope0(n.asNode()) } cached @@ -978,11 +1003,14 @@ module MakeImplCommon Lang> { predicate jumpStepCached(Node node1, Node node2) { jumpStep(node1, node2) } cached - predicate clearsContentCached(Node n, ContentSet c) { clearsContent(n, c) } + predicate clearsContentSet(NodeEx n, ContentSet c) { clearsContent(n.asNode(), c) } cached predicate expectsContentCached(Node n, ContentSet c) { expectsContent(n, c) } + cached + predicate expectsContentSet(NodeEx n, ContentSet c) { expectsContent(n.asNode(), c) } + cached predicate isUnreachableInCallCached(NodeRegion nr, DataFlowCall call) { isUnreachableInCall(nr, call) @@ -996,16 +1024,15 @@ module MakeImplCommon Lang> { } cached - predicate hiddenNode(Node n) { nodeIsHidden(n) } + predicate hiddenNode(NodeEx n) { + nodeIsHidden([n.asNode(), n.asParamReturnNode()]) + or + n instanceof TNodeImplicitRead + } cached - OutNodeExt getAnOutNodeExt(DataFlowCall call, ReturnKindExt k) { - result = getAnOutNode(call, k.(ValueReturnKind).getKind()) - or - exists(ArgNode arg | - result.(PostUpdateNode).getPreUpdateNode() = arg and - arg.argumentOf(call, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition()) - ) + OutNodeEx getAnOutNodeEx(DataFlowCall call, ReturnKindExt k) { + result.asNode() = getAnOutNodeExt(call, k) } pragma[nomagic] @@ -1016,22 +1043,22 @@ module MakeImplCommon Lang> { parameterValueFlowsToPreUpdate(p, n) and p.isParameterOf(_, pos) and k = TParamUpdate(pos) and - scope = getSecondLevelScopeCached(n) + scope = getSecondLevelScope0(n) ) } cached - predicate castNode(Node n) { n instanceof CastNode } + predicate flowCheckNode(NodeEx n) { + n.asNode() instanceof CastNode or + clearsContentSet(n, _) or + expectsContentSet(n, _) or + neverSkipInPathGraph(n.asNode()) + } cached - predicate castingNode(Node n) { - castNode(n) or - n instanceof ParamNode or - n instanceof OutNodeExt or - // For reads, `x.f`, we want to check that the tracked type after the read (which - // is obtained by popping the head of the access path stack) is compatible with - // the type of `x.f`. - readSet(_, _, n) + predicate castingNodeEx(NodeEx n) { + n.asNode() instanceof CastingNode or + exists(n.asParamReturnNode()) } cached @@ -1208,6 +1235,15 @@ module MakeImplCommon Lang> { ) } + /** + * Holds if `arg` is a possible argument to `p` in `call`, taking virtual + * dispatch into account. + */ + cached + predicate viableParamArgEx(DataFlowCall call, ParamNodeEx p, ArgNodeEx arg) { + viableParamArg(call, p.asNode(), arg.asNode()) + } + pragma[nomagic] private ReturnPosition viableReturnPos(DataFlowCall call, ReturnKindExt kind) { viableCallableExt(call) = result.getCallable() and @@ -1219,13 +1255,22 @@ module MakeImplCommon Lang> { * taking virtual dispatch into account. */ cached - predicate viableReturnPosOut(DataFlowCall call, ReturnPosition pos, Node out) { + predicate viableReturnPosOut(DataFlowCall call, ReturnPosition pos, OutNodeExt out) { exists(ReturnKindExt kind | pos = viableReturnPos(call, kind) and - out = kind.getAnOutNode(call) + out = getAnOutNodeExt(call, kind) ) } + /** + * Holds if a value at return position `pos` can be returned to `out` via `call`, + * taking virtual dispatch into account. + */ + cached + predicate viableReturnPosOutEx(DataFlowCall call, ReturnPosition pos, OutNodeEx out) { + viableReturnPosOut(call, pos, out.asNode()) + } + /** Provides predicates for calculating flow-through summaries. */ private module FlowThrough { /** @@ -1559,6 +1604,11 @@ module MakeImplCommon Lang> { cached predicate readSet(Node node1, ContentSet c, Node node2) { readStep(node1, c, node2) } + cached + predicate readEx(NodeEx node1, ContentSet c, NodeEx node2) { + readSet(pragma[only_bind_into](node1.asNode()), c, pragma[only_bind_into](node2.asNode())) + } + cached predicate storeSet( Node node1, ContentSet c, Node node2, DataFlowType contentType, DataFlowType containerType @@ -1587,11 +1637,13 @@ module MakeImplCommon Lang> { * been stored into, in order to handle cases like `x.f1.f2 = y`. */ cached - predicate store( - Node node1, Content c, Node node2, DataFlowType contentType, DataFlowType containerType + predicate storeEx( + NodeEx node1, Content c, NodeEx node2, DataFlowType contentType, DataFlowType containerType ) { exists(ContentSet cs | - c = cs.getAStoreContent() and storeSet(node1, cs, node2, contentType, containerType) + c = cs.getAStoreContent() and + storeSet(pragma[only_bind_into](node1.asNode()), cs, pragma[only_bind_into](node2.asNode()), + contentType, containerType) ) } @@ -1627,7 +1679,7 @@ module MakeImplCommon Lang> { } cached - predicate allowParameterReturnInSelfCached(ParamNode p) { allowParameterReturnInSelf(p) } + predicate allowParameterReturnInSelfEx(ParamNodeEx p) { allowParameterReturnInSelf(p.asNode()) } cached predicate paramMustFlow(ParamNode p, ArgNode arg) { localMustFlowStep+(p, arg) } @@ -1657,7 +1709,9 @@ module MakeImplCommon Lang> { string toString() { result = "Unreachable" } cached - predicate contains(Node n) { exists(NodeRegion nr | super.contains(nr) and nr.contains(n)) } + predicate contains(NodeEx n) { + exists(NodeRegion nr | super.contains(nr) and nr.contains(n.asNode())) + } cached DataFlowCallable getEnclosingCallable() { @@ -2209,7 +2263,15 @@ module MakeImplCommon Lang> { * A `Node` at which a cast can occur such that the type should be checked. */ class CastingNode extends NodeFinal { - CastingNode() { castingNode(this) } + CastingNode() { + this instanceof CastNode or + this instanceof ParamNode or + this instanceof OutNodeExt or + // For reads, `x.f`, we want to check that the tracked type after the read (which + // is obtained by popping the head of the access path stack) is compatible with + // the type of `x.f`. + readSet(_, _, this) + } } private predicate readStepWithTypes( @@ -2266,7 +2328,7 @@ module MakeImplCommon Lang> { } /** Holds if this call context makes `n` unreachable. */ - predicate unreachable(Node n) { ns.contains(n) } + predicate unreachable(NodeEx n) { ns.contains(n) } } private DataFlowCallable getNodeRegionEnclosingCallable(NodeRegion nr) { @@ -2307,6 +2369,16 @@ module MakeImplCommon Lang> { OutNodeExt() { outNodeExt(this) } } + pragma[nomagic] + OutNodeExt getAnOutNodeExt(DataFlowCall call, ReturnKindExt k) { + result = getAnOutNode(call, k.(ValueReturnKind).getKind()) + or + exists(ArgNode arg | + result.(PostUpdateNode).getPreUpdateNode() = arg and + arg.argumentOf(call, k.(ParamUpdateReturnKind).getAMatchingArgumentPosition()) + ) + } + /** * An extended return kind. A return kind describes how data can be returned * from a callable. This can either be through a returned value or an updated @@ -2317,7 +2389,7 @@ module MakeImplCommon Lang> { abstract string toString(); /** Gets a node corresponding to data flow out of `call`. */ - final OutNodeExt getAnOutNode(DataFlowCall call) { result = getAnOutNodeExt(call, this) } + final OutNodeEx getAnOutNodeEx(DataFlowCall call) { result = getAnOutNodeEx(call, this) } } class ValueReturnKind extends ReturnKindExt, TValueReturn { diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll index 4f6a88b6b28a..e7e588ff9ebe 100644 --- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll @@ -1355,7 +1355,7 @@ module Make< exists(DataFlowCall call, ReturnKindExt rk | result = summaryArgParam(call, arg, sc) and summaryReturnNodeExt(ret, pragma[only_bind_into](rk)) and - out = pragma[only_bind_into](rk).getAnOutNode(call) + out = getAnOutNodeExt(call, pragma[only_bind_into](rk)) ) } diff --git a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll index 2e22ae2c2ba8..5b53943ff832 100644 --- a/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll +++ b/shared/mad/codeql/mad/modelgenerator/internal/ModelGeneratorImpl.qll @@ -464,8 +464,10 @@ module MakeModelGenerator< predicate isAdditionalFlowStep( DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 ) { - exists(DataFlow::ContentSet c | - DataFlow::store(node1, c.getAStoreContent(), node2, _, _) and + exists(DataFlow::NodeEx n1, DataFlow::NodeEx n2, DataFlow::ContentSet c | + node1 = n1.asNode() and + node2 = n2.asNode() and + DataFlow::storeEx(n1, c.getAStoreContent(), n2, _, _) and isRelevantContent0(c) and ( state1 instanceof TaintRead and state2.(TaintStore).getStep() = 1