Skip to content

Commit 7ed16e0

Browse files
IdrissRioIdrissRio
committed
Java: Address review comment. Inline dataflow annotation
1 parent 0c651af commit 7ed16e0

File tree

3 files changed

+65
-36
lines changed

3 files changed

+65
-36
lines changed

‎java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,23 @@
1+
import java.lang.ScopedValue;
2+
13
public class ScopedValueFlowTest {
24
private static final ScopedValue<String> USER_CONTEXT = ScopedValue.newInstance();
35
private static final ScopedValue<String> SESSION_ID = ScopedValue.newInstance();
46

7+
public static String source(String label) {
8+
return "tainted";
9+
}
10+
11+
public static void sink(String value) {}
12+
513
public static void main(String[] args) {
6-
String userInput = args[0]; // source
14+
String userInput = source("");
715

816
// Test 1: Basic scoped value binding and retrieval
917
ScopedValue.where(USER_CONTEXT, userInput)
1018
.run(() -> {
1119
String value = USER_CONTEXT.get();
12-
sink(value); // should flag: tainted data reaches sink
20+
sink(value); // $ hasTaintFlow
1321
});
1422

1523
// Test 2: Multiple scoped value bindings with chaining
@@ -18,8 +26,8 @@ public static void main(String[] args) {
1826
.run(() -> {
1927
String user = USER_CONTEXT.get();
2028
String session = SESSION_ID.get();
21-
sink(user); // should flag: tainted data reaches sink
22-
sink(session); // should NOT flag
29+
sink(user); // $ hasTaintFlow
30+
sink(session); // safe - should NOT have taint flow
2331
});
2432

2533
ScopedValue.where(USER_CONTEXT, userInput)
@@ -28,12 +36,9 @@ public static void main(String[] args) {
2836
ScopedValue.where(USER_CONTEXT, "safe-two")
2937
.run(() -> {
3038
String inner = USER_CONTEXT.get();
31-
sink(inner); // False Positive: currently flags (model limitation
39+
sink(inner); // $ SPURIOUS: hasTaintFlow
3240
});
33-
sink(outer); // should flag: tainted data reaches sink
41+
sink(outer); // $ hasTaintFlow
3442
});
3543
}
36-
37-
public static void sink(String s) {
38-
}
3944
}

‎java/ql/test/library-tests/dataflow/scoped-values/test.expected

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,48 @@
1-
| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:12:22:12:26 | value |
2-
| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:21:22:21:25 | user |
3-
| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:31:30:31:34 | inner |
4-
| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:33:22:33:26 | outer |
1+
models
2+
| 1 | Summary: java.lang; ScopedValue; false; where; (ScopedValue,Object); ; Argument[1]; Argument[0].SyntheticField[java.lang.ScopedValue.boundValue]; value; manual |
3+
| 2 | Summary: java.lang; ScopedValue; true; get; (); ; Argument[this].SyntheticField[java.lang.ScopedValue.boundValue]; ReturnValue; value; manual |
4+
edges
5+
| ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:19:32:19:43 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | |
6+
| ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:27:31:27:42 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | |
7+
| ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:35:32:35:43 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | |
8+
| ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:38:40:38:51 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | |
9+
| ScopedValueFlowTest.java:14:28:14:37 | source(...) : String | ScopedValueFlowTest.java:17:41:17:49 | userInput : String | provenance | |
10+
| ScopedValueFlowTest.java:14:28:14:37 | source(...) : String | ScopedValueFlowTest.java:24:41:24:49 | userInput : String | provenance | |
11+
| ScopedValueFlowTest.java:14:28:14:37 | source(...) : String | ScopedValueFlowTest.java:33:41:33:49 | userInput : String | provenance | |
12+
| ScopedValueFlowTest.java:17:27:17:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | |
13+
| ScopedValueFlowTest.java:17:41:17:49 | userInput : String | ScopedValueFlowTest.java:17:27:17:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | MaD:1 |
14+
| ScopedValueFlowTest.java:19:32:19:43 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:19:32:19:49 | get(...) : String | provenance | MaD:2 |
15+
| ScopedValueFlowTest.java:19:32:19:49 | get(...) : String | ScopedValueFlowTest.java:20:22:20:26 | value | provenance | |
16+
| ScopedValueFlowTest.java:24:27:24:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | |
17+
| ScopedValueFlowTest.java:24:41:24:49 | userInput : String | ScopedValueFlowTest.java:24:27:24:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | MaD:1 |
18+
| ScopedValueFlowTest.java:27:31:27:42 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:27:31:27:48 | get(...) : String | provenance | MaD:2 |
19+
| ScopedValueFlowTest.java:27:31:27:48 | get(...) : String | ScopedValueFlowTest.java:29:22:29:25 | user | provenance | |
20+
| ScopedValueFlowTest.java:33:27:33:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | |
21+
| ScopedValueFlowTest.java:33:41:33:49 | userInput : String | ScopedValueFlowTest.java:33:27:33:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | provenance | MaD:1 |
22+
| ScopedValueFlowTest.java:35:32:35:43 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:35:32:35:49 | get(...) : String | provenance | MaD:2 |
23+
| ScopedValueFlowTest.java:35:32:35:49 | get(...) : String | ScopedValueFlowTest.java:41:22:41:26 | outer | provenance | |
24+
| ScopedValueFlowTest.java:38:40:38:51 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | ScopedValueFlowTest.java:38:40:38:57 | get(...) : String | provenance | MaD:2 |
25+
| ScopedValueFlowTest.java:38:40:38:57 | get(...) : String | ScopedValueFlowTest.java:39:30:39:34 | inner | provenance | |
26+
nodes
27+
| ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String |
28+
| ScopedValueFlowTest.java:14:28:14:37 | source(...) : String | semmle.label | source(...) : String |
29+
| ScopedValueFlowTest.java:17:27:17:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String |
30+
| ScopedValueFlowTest.java:17:41:17:49 | userInput : String | semmle.label | userInput : String |
31+
| ScopedValueFlowTest.java:19:32:19:43 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String |
32+
| ScopedValueFlowTest.java:19:32:19:49 | get(...) : String | semmle.label | get(...) : String |
33+
| ScopedValueFlowTest.java:20:22:20:26 | value | semmle.label | value |
34+
| ScopedValueFlowTest.java:24:27:24:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String |
35+
| ScopedValueFlowTest.java:24:41:24:49 | userInput : String | semmle.label | userInput : String |
36+
| ScopedValueFlowTest.java:27:31:27:42 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String |
37+
| ScopedValueFlowTest.java:27:31:27:48 | get(...) : String | semmle.label | get(...) : String |
38+
| ScopedValueFlowTest.java:29:22:29:25 | user | semmle.label | user |
39+
| ScopedValueFlowTest.java:33:27:33:38 | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT [post update] : ScopedValue [java.lang.ScopedValue.boundValue] : String |
40+
| ScopedValueFlowTest.java:33:41:33:49 | userInput : String | semmle.label | userInput : String |
41+
| ScopedValueFlowTest.java:35:32:35:43 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String |
42+
| ScopedValueFlowTest.java:35:32:35:49 | get(...) : String | semmle.label | get(...) : String |
43+
| ScopedValueFlowTest.java:38:40:38:51 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String |
44+
| ScopedValueFlowTest.java:38:40:38:57 | get(...) : String | semmle.label | get(...) : String |
45+
| ScopedValueFlowTest.java:39:30:39:34 | inner | semmle.label | inner |
46+
| ScopedValueFlowTest.java:41:22:41:26 | outer | semmle.label | outer |
47+
subpaths
48+
testFailures

‎java/ql/test/library-tests/dataflow/scoped-values/test.ql

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,4 @@
11
import java
2-
import semmle.code.java.dataflow.TaintTracking
3-
4-
module Config implements DataFlow::ConfigSig {
5-
predicate isSource(DataFlow::Node n) {
6-
exists(ArrayAccess aa |
7-
aa.getArray().(VarAccess).getVariable().hasName("args") and
8-
n.asExpr() = aa
9-
)
10-
}
11-
12-
predicate isSink(DataFlow::Node n) {
13-
exists(MethodCall ma |
14-
ma.getMethod().hasName("sink") and
15-
n.asExpr() = ma.getAnArgument()
16-
)
17-
}
18-
}
19-
20-
module Flow = TaintTracking::Global<Config>;
21-
22-
from DataFlow::Node src, DataFlow::Node sink
23-
where Flow::flow(src, sink)
24-
select src, sink
2+
import utils.test.InlineFlowTest
3+
import TaintFlowTest<DefaultFlowConfig>
4+
import TaintFlow::PathGraph

0 commit comments

Comments
 (0)