From cb0fe40b850f1e11947e2a7eb1920dfadd4483de Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 20 Aug 2024 13:35:02 +0200 Subject: [PATCH] Shared: prevent use-use flow through implicit reads --- .../codeql/dataflow/internal/DataFlowImpl.qll | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll index 6eb1efc2b3da..bfdf7c55f3d2 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll @@ -179,6 +179,11 @@ module MakeImpl Lang> { Node asNode() { this = TNodeNormal(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, true) + } + predicate isImplicitReadNode(Node n, boolean hasRead) { this = TNodeImplicitRead(n, hasRead) } ParameterNode asParamReturnNode() { this = TParamReturnNode(result, _) } @@ -247,6 +252,16 @@ module MakeImpl Lang> { ReturnKindExt getKind() { result = pos.getKind() } } + /** If `node` corresponds to a sink, gets the normal node for that sink. */ + pragma[nomagic] + private NodeEx toNormalSinkNodeEx(NodeEx node) { + exists(Node n | + node.asNodeOrImplicitRead() = n and + (Config::isSink(n) or Config::isSink(n, _)) and + result.asNode() = n + ) + } + private predicate inBarrier(NodeEx node) { exists(Node n | node.asNode() = n and @@ -266,7 +281,7 @@ module MakeImpl Lang> { private predicate outBarrier(NodeEx node) { exists(Node n | - node.asNode() = n and + node.asNodeOrImplicitRead() = n and Config::isBarrierOut(n) | Config::isSink(n, _) @@ -278,7 +293,7 @@ module MakeImpl Lang> { pragma[nomagic] private predicate outBarrier(NodeEx node, FlowState state) { exists(Node n | - node.asNode() = n and + node.asNodeOrImplicitRead() = n and Config::isBarrierOut(n, state) | Config::isSink(n, state) @@ -324,7 +339,7 @@ module MakeImpl Lang> { pragma[nomagic] private predicate sinkNodeWithState(NodeEx node, FlowState state) { - Config::isSink(node.asNode(), state) and + Config::isSink(node.asNodeOrImplicitRead(), state) and not fullBarrier(node) and not stateBarrier(node, state) } @@ -386,7 +401,7 @@ module MakeImpl Lang> { */ private predicate additionalLocalFlowStep(NodeEx node1, NodeEx node2, string model) { exists(Node n1, Node n2 | - node1.asNode() = n1 and + node1.asNodeOrImplicitRead() = n1 and node2.asNode() = n2 and Config::isAdditionalFlowStep(pragma[only_bind_into](n1), pragma[only_bind_into](n2), model) and getNodeEnclosingCallable(n1) = getNodeEnclosingCallable(n2) and @@ -395,8 +410,7 @@ module MakeImpl Lang> { or exists(Node n | node1.isImplicitReadNode(n, true) and - node2.asNode() = n and - not fullBarrier(node2) and + node2.isImplicitReadNode(n, false) and model = "" ) } @@ -405,7 +419,7 @@ module MakeImpl Lang> { NodeEx node1, FlowState s1, NodeEx node2, FlowState s2 ) { exists(Node n1, Node n2 | - node1.asNode() = n1 and + node1.asNodeOrImplicitRead() = n1 and node2.asNode() = n2 and Config::isAdditionalFlowStep(pragma[only_bind_into](n1), s1, pragma[only_bind_into](n2), s2) and getNodeEnclosingCallable(n1) = getNodeEnclosingCallable(n2) and @@ -431,7 +445,7 @@ module MakeImpl Lang> { */ private predicate additionalJumpStep(NodeEx node1, NodeEx node2, string model) { exists(Node n1, Node n2 | - node1.asNode() = n1 and + node1.asNodeOrImplicitRead() = n1 and node2.asNode() = n2 and Config::isAdditionalFlowStep(pragma[only_bind_into](n1), pragma[only_bind_into](n2), model) and getNodeEnclosingCallable(n1) != getNodeEnclosingCallable(n2) and @@ -442,7 +456,7 @@ module MakeImpl Lang> { private predicate additionalJumpStateStep(NodeEx node1, FlowState s1, NodeEx node2, FlowState s2) { exists(Node n1, Node n2 | - node1.asNode() = n1 and + node1.asNodeOrImplicitRead() = n1 and node2.asNode() = n2 and Config::isAdditionalFlowStep(pragma[only_bind_into](n1), s1, pragma[only_bind_into](n2), s2) and getNodeEnclosingCallable(n1) != getNodeEnclosingCallable(n2) and @@ -735,7 +749,7 @@ module MakeImpl Lang> { additional predicate sinkNode(NodeEx node, FlowState state) { fwdFlow(node) and fwdFlowState(state) and - Config::isSink(node.asNode()) + Config::isSink(node.asNodeOrImplicitRead()) or fwdFlow(node) and fwdFlowState(state) and @@ -1058,7 +1072,7 @@ module MakeImpl Lang> { private predicate sinkModel(NodeEx node, string model) { sinkNode(node, _) and - exists(Node n | n = node.asNode() | + exists(Node n | n = node.asNodeOrImplicitRead() | knownSinkModel(n, model) or not knownSinkModel(n, _) and model = "" @@ -3852,7 +3866,7 @@ module MakeImpl Lang> { TPathNodeSink(NodeEx node, FlowState state) { exists(PathNodeMid sink | sink.isAtSink(_) and - node = sink.getNodeEx() and + node = toNormalSinkNodeEx(sink.getNodeEx()) and state = sink.getState() ) } or @@ -4339,7 +4353,7 @@ module MakeImpl Lang> { PathNodeSink projectToSink(string model) { this.isAtSink(model) and - result.getNodeEx() = node and + result.getNodeEx() = toNormalSinkNodeEx(node) and result.getState() = state } } @@ -5231,7 +5245,7 @@ module MakeImpl Lang> { private predicate revSinkNode(NodeEx node, FlowState state) { sinkNodeWithState(node, state) or - Config::isSink(node.asNode()) and + Config::isSink(node.asNodeOrImplicitRead()) and relevantState(state) and not fullBarrier(node) and not stateBarrier(node, state)