Skip to content

Commit f2968f4

Browse files
committed
Shared: Ensure subpath-induced edges are handled properly
Argument-passing and flow-through edges are present in 'edges' in addition to 'subpaths', but the implementation didn't take this into account.
1 parent 0edb306 commit f2968f4

File tree

2 files changed

+10
-5
lines changed
  • java/ql/test/library-tests/dataflow/deduplicate-path-graph
  • shared/dataflow/codeql/dataflow

2 files changed

+10
-5
lines changed

java/ql/test/library-tests/dataflow/deduplicate-path-graph/test.ql

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,10 @@ predicate reachableFromPropagate(Graph::PathNode node, string state, boolean cal
4747
node.getNode().asExpr() = propagateCall(state) and call = false
4848
or
4949
exists(Graph::PathNode prev | reachableFromPropagate(prev, state, call) |
50-
Graph::edges(prev, node, _, _)
50+
Graph::edges(prev, node, _, _) and
51+
not Graph::subpaths(prev, node, _, _) // argument-passing edges are handled separately
5152
or
52-
Graph::subpaths(prev, _, _, node) // arg -> out
53+
Graph::subpaths(prev, _, _, node) // arg -> out (should be included in 'edges' but handle the case here for clarity)
5354
)
5455
or
5556
exists(Graph::PathNode prev |

shared/dataflow/codeql/dataflow/DataFlow.qll

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -935,11 +935,15 @@ module DataFlowMake<LocationSig Location, InputSig<Location> Lang> {
935935
)
936936
}
937937

938-
/** Gets a successor of `node` including subpath flow-through. */
938+
/** Gets a successor of `node`, including subpath flow-through, but not enter or exit subpath steps. */
939939
InputPathNode stepEx(InputPathNode node) {
940-
step(node, result, _, _)
940+
step(node, result, _, _) and
941+
not result = enterSubpathStep(node) and
942+
not result = exitSubpathStep(node)
941943
or
942-
subpathStep(node, _, _, result) // assuming the input is pruned properly, all subpaths have flow-through
944+
// Assuming the input is pruned properly, all subpaths have flow-through.
945+
// This step should be in 'step' as well, but include it here for clarity as we rely on it.
946+
subpathStep(node, _, _, result)
943947
}
944948

945949
InputPathNode enterSubpathStep(InputPathNode node) { subpathStep(node, result, _, _) }

0 commit comments

Comments
 (0)