Skip to content

Commit 0eb543e

Browse files
committed
Java: add test for spurious flow from path graph deduplication
1 parent 8efdc2d commit 0eb543e

File tree

3 files changed

+184
-0
lines changed

3 files changed

+184
-0
lines changed
Lines changed: 34 additions & 0 deletions
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+
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
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:33:18:37 | step0 : String | semmle.label | step0 : String |
11+
| A.java:19:20:19:38 | apply(...) : String | semmle.label | apply(...) : String |
12+
| A.java:19:33:19:37 | step1 : String | semmle.label | step1 : String |
13+
| A.java:21:10:21:14 | step2 | semmle.label | step2 |
14+
| A.java:26:8:26:8 | x : String | semmle.label | x : String |
15+
| A.java:26:13:26:81 | ...?...:... : String | semmle.label | ...?...:... : String |
16+
| A.java:26:35:26:56 | propagateState(...) : String | semmle.label | propagateState(...) : String |
17+
| A.java:26:50:26:50 | x : String | semmle.label | x : String |
18+
| A.java:26:60:26:81 | propagateState(...) : String | semmle.label | propagateState(...) : String |
19+
| A.java:26:75:26:75 | x : String | semmle.label | x : String |
20+
| A.java:28:20:28:27 | source(...) : String | semmle.label | source(...) : String |
21+
| A.java:29:20:29:38 | apply(...) : String | semmle.label | apply(...) : String |
22+
| A.java:29:33:29:37 | step0 : String | semmle.label | step0 : String |
23+
| A.java:30:20:30:38 | apply(...) : String | semmle.label | apply(...) : String |
24+
| A.java:30:33:30:37 | step1 : String | semmle.label | step1 : String |
25+
| A.java:32:10:32:14 | step2 | semmle.label | step2 |
26+
edges
27+
| A.java:14:9:14:9 | x : String | A.java:14:29:14:29 | x : String |
28+
| A.java:14:29:14:29 | x : String | A.java:14:14:14:35 | propagateState(...) : String |
29+
| A.java:15:9:15:9 | x : String | A.java:15:29:15:29 | x : String |
30+
| A.java:15:29:15:29 | x : String | A.java:15:14:15:35 | propagateState(...) : String |
31+
| A.java:17:20:17:27 | source(...) : String | A.java:18:33:18:37 | step0 : String |
32+
| A.java:18:20:18:38 | apply(...) : String | A.java:19:33:19:37 | step1 : String |
33+
| A.java:18:33:18:37 | step0 : String | A.java:14:9:14:9 | x : String |
34+
| A.java:18:33:18:37 | step0 : String | A.java:15:9:15:9 | x : String |
35+
| A.java:18:33:18:37 | step0 : String | A.java:18:20:18:38 | apply(...) : String |
36+
| A.java:19:20:19:38 | apply(...) : String | A.java:21:10:21:14 | step2 |
37+
| A.java:19:33:19:37 | step1 : String | A.java:14:9:14:9 | x : String |
38+
| A.java:19:33:19:37 | step1 : String | A.java:15:9:15:9 | x : String |
39+
| A.java:19:33:19:37 | step1 : String | A.java:19:20:19:38 | apply(...) : String |
40+
| A.java:26:8:26:8 | x : String | A.java:26:50:26:50 | x : String |
41+
| A.java:26:8:26:8 | x : String | A.java:26:75:26:75 | x : String |
42+
| A.java:26:35:26:56 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String |
43+
| A.java:26:50:26:50 | x : String | A.java:26:35:26:56 | propagateState(...) : String |
44+
| A.java:26:60:26:81 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String |
45+
| A.java:26:75:26:75 | x : String | A.java:26:60:26:81 | propagateState(...) : String |
46+
| A.java:28:20:28:27 | source(...) : String | A.java:29:33:29:37 | step0 : String |
47+
| A.java:29:20:29:38 | apply(...) : String | A.java:30:33:30:37 | step1 : String |
48+
| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String |
49+
| A.java:29:33:29:37 | step0 : String | A.java:29:20:29:38 | apply(...) : String |
50+
| A.java:30:20:30:38 | apply(...) : String | A.java:32:10:32:14 | step2 |
51+
| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String |
52+
| A.java:30:33:30:37 | step1 : String | A.java:30:20:30:38 | apply(...) : String |
53+
subpaths
54+
| 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 |
55+
| 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 |
56+
| 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 |
57+
| 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 |
58+
| 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 |
59+
| 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 |
60+
spuriousFlow
61+
| A.java:14:14:14:35 | propagateState(...) : String | B | A |
62+
| A.java:15:14:15:35 | propagateState(...) : String | A | B |
63+
| A.java:26:35:26:56 | propagateState(...) : String | B | A |
64+
| A.java:26:60:26:81 | propagateState(...) : String | A | B |
65+
#select
66+
| A.java:17:20:17:27 | source(...) : String | A.java:17:20:17:27 | source(...) : String | A.java:21:10:21:14 | step2 | Flow |
67+
| A.java:28:20:28:27 | source(...) : String | A.java:28:20:28:27 | source(...) : String | A.java:32:10:32:14 | step2 | Flow |
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import java
6+
import semmle.code.java.dataflow.DataFlow
7+
import DataFlow
8+
9+
MethodAccess 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().(MethodAccess).getMethod().getName() = "source" and state = ["A", "B"]
19+
}
20+
21+
predicate isSink(Node n, FlowState state) {
22+
n.asExpr() = any(MethodAccess 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(MethodAccess 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)
51+
or
52+
Graph::subpaths(prev, _, _, node) // arg -> out
53+
)
54+
or
55+
exists(Graph::PathNode prev |
56+
reachableFromPropagate(prev, state, _) and
57+
Graph::subpaths(prev, node, _, _) and // arg -> parameter
58+
call = true
59+
)
60+
or
61+
exists(Graph::PathNode prev |
62+
reachableFromPropagate(prev, state, call) and
63+
Graph::subpaths(_, _, prev, node) and // return -> out
64+
call = false
65+
)
66+
}
67+
68+
/**
69+
* Holds if `node` is the return value of a `propagateState` call that appears to be reachable
70+
* with a different state than the one propagated by the call, indicating spurious flow resulting from
71+
* merging path nodes.
72+
*/
73+
query predicate spuriousFlow(Graph::PathNode node, string state1, string state2) {
74+
reachableFromPropagate(node, state1, _) and
75+
node.getNode().asExpr() = propagateCall(state2) and
76+
state1 != state2
77+
}
78+
79+
import Graph::PathGraph
80+
81+
from Graph::PathNode source, Graph::PathNode sink
82+
where TestFlow::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode())
83+
select source, source, sink, "Flow"

0 commit comments

Comments
 (0)