Skip to content

Commit be939dc

Browse files
asgerfasgerf
authored
Merge pull request #14350 from asgerf/shared/deduplicate-path-graph
Shared: Add DataFlow::DeduplicatePathGraph
2 parents 00688eb + e34fbc8 commit be939dc

File tree

7 files changed

+469
-80
lines changed

7 files changed

+469
-80
lines changed

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

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import java.util.function.Function;
2+
3+
class A {
4+
String source() { return ""; }
5+
6+
void sink(String s) { }
7+
8+
String propagateState(String s, String state) {
9+
return "";
10+
}
11+
12+
void foo() {
13+
Function<String, String> lambda = Math.random() > 0.5
14+
? x -> propagateState(x, "A")
15+
: x -> propagateState(x, "B");
16+
17+
String step0 = source();
18+
String step1 = lambda.apply(step0);
19+
String step2 = lambda.apply(step1);
20+
21+
sink(step2);
22+
}
23+
24+
void bar() {
25+
Function<String, String> lambda =
26+
(x -> Math.random() > 0.5 ? propagateState(x, "A") : propagateState(x, "B"));
27+
28+
String step0 = source();
29+
String step1 = lambda.apply(step0);
30+
String step2 = lambda.apply(step1);
31+
32+
sink(step2);
33+
}
34+
}

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

+79
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
nodes
2+
| A.java:14:9:14:9 | x : String | semmle.label | x : String |
3+
| A.java:14:14:14:35 | propagateState(...) : String | semmle.label | propagateState(...) : String |
4+
| A.java:14:29:14:29 | x : String | semmle.label | x : String |
5+
| A.java:15:9:15:9 | x : String | semmle.label | x : String |
6+
| A.java:15:14:15:35 | propagateState(...) : String | semmle.label | propagateState(...) : String |
7+
| A.java:15:29:15:29 | x : String | semmle.label | x : String |
8+
| A.java:17:20:17:27 | source(...) : String | semmle.label | source(...) : String |
9+
| A.java:18:20:18:38 | apply(...) : String | semmle.label | apply(...) : String |
10+
| A.java:18:20:18:38 | apply(...) : String | semmle.label | apply(...) : String |
11+
| A.java:18:33:18:37 | step0 : String | semmle.label | step0 : String |
12+
| A.java:19:20:19:38 | apply(...) : String | semmle.label | apply(...) : String |
13+
| A.java:19:33:19:37 | step1 : String | semmle.label | step1 : String |
14+
| A.java:19:33:19:37 | step1 : String | semmle.label | step1 : String |
15+
| A.java:21:10:21:14 | step2 | semmle.label | step2 |
16+
| A.java:26:8:26:8 | x : String | semmle.label | x : String |
17+
| A.java:26:8:26:8 | x : String | semmle.label | x : String |
18+
| A.java:26:13:26:81 | ...?...:... : String | semmle.label | ...?...:... : String |
19+
| A.java:26:13:26:81 | ...?...:... : String | semmle.label | ...?...:... : String |
20+
| A.java:26:35:26:56 | propagateState(...) : String | semmle.label | propagateState(...) : String |
21+
| A.java:26:50:26:50 | x : String | semmle.label | x : String |
22+
| A.java:26:60:26:81 | propagateState(...) : String | semmle.label | propagateState(...) : String |
23+
| A.java:26:75:26:75 | x : String | semmle.label | x : String |
24+
| A.java:28:20:28:27 | source(...) : String | semmle.label | source(...) : String |
25+
| A.java:29:20:29:38 | apply(...) : String | semmle.label | apply(...) : String |
26+
| A.java:29:20:29:38 | apply(...) : String | semmle.label | apply(...) : String |
27+
| A.java:29:33:29:37 | step0 : String | semmle.label | step0 : String |
28+
| A.java:30:20:30:38 | apply(...) : String | semmle.label | apply(...) : String |
29+
| A.java:30:33:30:37 | step1 : String | semmle.label | step1 : String |
30+
| A.java:30:33:30:37 | step1 : String | semmle.label | step1 : String |
31+
| A.java:32:10:32:14 | step2 | semmle.label | step2 |
32+
edges
33+
| A.java:14:9:14:9 | x : String | A.java:14:29:14:29 | x : String | provenance | |
34+
| A.java:14:29:14:29 | x : String | A.java:14:14:14:35 | propagateState(...) : String | provenance | Config |
35+
| A.java:15:9:15:9 | x : String | A.java:15:29:15:29 | x : String | provenance | |
36+
| A.java:15:29:15:29 | x : String | A.java:15:14:15:35 | propagateState(...) : String | provenance | Config |
37+
| A.java:17:20:17:27 | source(...) : String | A.java:18:33:18:37 | step0 : String | provenance | |
38+
| A.java:18:20:18:38 | apply(...) : String | A.java:19:33:19:37 | step1 : String | provenance | |
39+
| A.java:18:20:18:38 | apply(...) : String | A.java:19:33:19:37 | step1 : String | provenance | |
40+
| A.java:18:33:18:37 | step0 : String | A.java:14:9:14:9 | x : String | provenance | |
41+
| A.java:18:33:18:37 | step0 : String | A.java:15:9:15:9 | x : String | provenance | |
42+
| A.java:18:33:18:37 | step0 : String | A.java:18:20:18:38 | apply(...) : String | provenance | Config |
43+
| A.java:18:33:18:37 | step0 : String | A.java:18:20:18:38 | apply(...) : String | provenance | Config |
44+
| A.java:19:20:19:38 | apply(...) : String | A.java:21:10:21:14 | step2 | provenance | |
45+
| A.java:19:33:19:37 | step1 : String | A.java:14:9:14:9 | x : String | provenance | |
46+
| A.java:19:33:19:37 | step1 : String | A.java:15:9:15:9 | x : String | provenance | |
47+
| A.java:19:33:19:37 | step1 : String | A.java:19:20:19:38 | apply(...) : String | provenance | Config |
48+
| A.java:19:33:19:37 | step1 : String | A.java:19:20:19:38 | apply(...) : String | provenance | Config |
49+
| A.java:26:8:26:8 | x : String | A.java:26:50:26:50 | x : String | provenance | |
50+
| A.java:26:8:26:8 | x : String | A.java:26:75:26:75 | x : String | provenance | |
51+
| A.java:26:35:26:56 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String | provenance | |
52+
| A.java:26:50:26:50 | x : String | A.java:26:35:26:56 | propagateState(...) : String | provenance | Config |
53+
| A.java:26:60:26:81 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String | provenance | |
54+
| A.java:26:75:26:75 | x : String | A.java:26:60:26:81 | propagateState(...) : String | provenance | Config |
55+
| A.java:28:20:28:27 | source(...) : String | A.java:29:33:29:37 | step0 : String | provenance | |
56+
| A.java:29:20:29:38 | apply(...) : String | A.java:30:33:30:37 | step1 : String | provenance | |
57+
| A.java:29:20:29:38 | apply(...) : String | A.java:30:33:30:37 | step1 : String | provenance | |
58+
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | provenance | |
59+
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | provenance | |
60+
| A.java:29:33:29:37 | step0 : String | A.java:29:20:29:38 | apply(...) : String | provenance | Config |
61+
| A.java:29:33:29:37 | step0 : String | A.java:29:20:29:38 | apply(...) : String | provenance | Config |
62+
| A.java:30:20:30:38 | apply(...) : String | A.java:32:10:32:14 | step2 | provenance | |
63+
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | provenance | |
64+
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | provenance | |
65+
| A.java:30:33:30:37 | step1 : String | A.java:30:20:30:38 | apply(...) : String | provenance | Config |
66+
| A.java:30:33:30:37 | step1 : String | A.java:30:20:30:38 | apply(...) : String | provenance | Config |
67+
subpaths
68+
| A.java:18:33:18:37 | step0 : String | A.java:14:9:14:9 | x : String | A.java:14:14:14:35 | propagateState(...) : String | A.java:18:20:18:38 | apply(...) : String |
69+
| A.java:18:33:18:37 | step0 : String | A.java:15:9:15:9 | x : String | A.java:15:14:15:35 | propagateState(...) : String | A.java:18:20:18:38 | apply(...) : String |
70+
| A.java:19:33:19:37 | step1 : String | A.java:14:9:14:9 | x : String | A.java:14:14:14:35 | propagateState(...) : String | A.java:19:20:19:38 | apply(...) : String |
71+
| A.java:19:33:19:37 | step1 : String | A.java:15:9:15:9 | x : String | A.java:15:14:15:35 | propagateState(...) : String | A.java:19:20:19:38 | apply(...) : String |
72+
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:29:20:29:38 | apply(...) : String |
73+
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:29:20:29:38 | apply(...) : String |
74+
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:30:20:30:38 | apply(...) : String |
75+
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:30:20:30:38 | apply(...) : String |
76+
spuriousFlow
77+
#select
78+
| A.java:17:20:17:27 | source(...) : String | A.java:17:20:17:27 | source(...) : String | A.java:21:10:21:14 | step2 | Flow |
79+
| A.java:28:20:28:27 | source(...) : String | A.java:28:20:28:27 | source(...) : String | A.java:32:10:32:14 | step2 | Flow |

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

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import java
6+
import semmle.code.java.dataflow.DataFlow
7+
import DataFlow
8+
9+
MethodCall propagateCall(string state) {
10+
result.getMethod().getName() = "propagateState" and
11+
state = result.getArgument(1).(StringLiteral).getValue()
12+
}
13+
14+
module TestConfig implements StateConfigSig {
15+
class FlowState = string;
16+
17+
predicate isSource(Node n, FlowState state) {
18+
n.asExpr().(MethodCall).getMethod().getName() = "source" and state = ["A", "B"]
19+
}
20+
21+
predicate isSink(Node n, FlowState state) {
22+
n.asExpr() = any(MethodCall acc | acc.getMethod().getName() = "sink").getAnArgument() and
23+
state = ["A", "B"]
24+
}
25+
26+
predicate isAdditionalFlowStep(Node node1, FlowState state1, Node node2, FlowState state2) {
27+
exists(MethodCall call |
28+
call = propagateCall(state1) and
29+
state2 = state1 and
30+
node1.asExpr() = call.getArgument(0) and
31+
node2.asExpr() = call
32+
)
33+
}
34+
}
35+
36+
module TestFlow = DataFlow::GlobalWithState<TestConfig>;
37+
38+
module Graph = DataFlow::DeduplicatePathGraph<TestFlow::PathNode, TestFlow::PathGraph>;
39+
40+
/**
41+
* Holds if `node` is reachable from a call to `propagateState` with the given `state` argument.
42+
* `call` indicates if a call step was taken (i.e. into a subpath).
43+
*
44+
* We use this to check if one `propagateState` can flow out of another, which is not allowed.
45+
*/
46+
predicate reachableFromPropagate(Graph::PathNode node, string state, boolean call) {
47+
node.getNode().asExpr() = propagateCall(state) and call = false
48+
or
49+
exists(Graph::PathNode prev | reachableFromPropagate(prev, state, call) |
50+
Graph::edges(prev, node, _, _) and
51+
not Graph::subpaths(prev, node, _, _) // argument-passing edges are handled separately
52+
or
53+
Graph::subpaths(prev, _, _, node) // arg -> out (should be included in 'edges' but handle the case here for clarity)
54+
)
55+
or
56+
exists(Graph::PathNode prev |
57+
reachableFromPropagate(prev, state, _) and
58+
Graph::subpaths(prev, node, _, _) and // arg -> parameter
59+
call = true
60+
)
61+
or
62+
exists(Graph::PathNode prev |
63+
reachableFromPropagate(prev, state, call) and
64+
Graph::subpaths(_, _, prev, node) and // return -> out
65+
call = false
66+
)
67+
}
68+
69+
/**
70+
* Holds if `node` is the return value of a `propagateState` call that appears to be reachable
71+
* with a different state than the one propagated by the call, indicating spurious flow resulting from
72+
* merging path nodes.
73+
*/
74+
query predicate spuriousFlow(Graph::PathNode node, string state1, string state2) {
75+
reachableFromPropagate(node, state1, _) and
76+
node.getNode().asExpr() = propagateCall(state2) and
77+
state1 != state2
78+
}
79+
80+
import Graph::PathGraph
81+
82+
from Graph::PathNode source, Graph::PathNode sink
83+
where TestFlow::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode())
84+
select source, source, sink, "Flow"

‎ruby/ql/src/queries/security/cwe-094/CodeInjection.ql

+4-15
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,9 @@
1616

1717
private import codeql.ruby.AST
1818
private import codeql.ruby.security.CodeInjectionQuery
19-
import CodeInjectionFlow::PathGraph
19+
import DataFlow::DeduplicatePathGraph<CodeInjectionFlow::PathNode, CodeInjectionFlow::PathGraph>
2020

21-
from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Source sourceNode
22-
where
23-
CodeInjectionFlow::flowPath(source, sink) and
24-
sourceNode = source.getNode() and
25-
// removing duplications of the same path, but different flow-labels.
26-
sink =
27-
min(CodeInjectionFlow::PathNode otherSink |
28-
CodeInjectionFlow::flowPath(any(CodeInjectionFlow::PathNode s | s.getNode() = sourceNode),
29-
otherSink) and
30-
otherSink.getNode() = sink.getNode()
31-
|
32-
otherSink order by otherSink.getState().getStringRepresentation()
33-
)
34-
select sink.getNode(), source, sink, "This code execution depends on a $@.", sourceNode,
21+
from PathNode source, PathNode sink
22+
where CodeInjectionFlow::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode())
23+
select sink.getNode(), source, sink, "This code execution depends on a $@.", source.getNode(),
3524
"user-provided value"

0 commit comments

Comments
 (0)