From 0159f5b422f311b287d97e4ac6a012d3a2d16a8d Mon Sep 17 00:00:00 2001 From: idrissrio Date: Mon, 1 Sep 2025 17:40:00 +0200 Subject: [PATCH 1/6] Java: Add failing test for Scoped Values --- .../scoped-values/ScopedValueFlowTest.java | 39 +++++++++++++++++++ .../dataflow/scoped-values/options | 1 + .../dataflow/scoped-values/test.expected | 0 .../dataflow/scoped-values/test.ql | 24 ++++++++++++ 4 files changed, 64 insertions(+) create mode 100644 java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java create mode 100644 java/ql/test/library-tests/dataflow/scoped-values/options create mode 100644 java/ql/test/library-tests/dataflow/scoped-values/test.expected create mode 100644 java/ql/test/library-tests/dataflow/scoped-values/test.ql diff --git a/java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java b/java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java new file mode 100644 index 00000000000..d71233f7dc8 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java @@ -0,0 +1,39 @@ +public class ScopedValueFlowTest { + private static final ScopedValue USER_CONTEXT = ScopedValue.newInstance(); + private static final ScopedValue SESSION_ID = ScopedValue.newInstance(); + + public static void main(String[] args) { + String userInput = args[0]; // source + + // Test 1: Basic scoped value binding and retrieval + ScopedValue.where(USER_CONTEXT, userInput) + .run(() -> { + String value = USER_CONTEXT.get(); + sink(value); // should flag: tainted data reaches sink + }); + + // Test 2: Multiple scoped value bindings with chaining + ScopedValue.where(USER_CONTEXT, userInput) + .where(SESSION_ID, "safe-one") + .run(() -> { + String user = USER_CONTEXT.get(); + String session = SESSION_ID.get(); + sink(user); // should flag: tainted data reaches sink + sink(session); // should NOT flag + }); + + ScopedValue.where(USER_CONTEXT, userInput) + .run(() -> { + String outer = USER_CONTEXT.get(); + ScopedValue.where(USER_CONTEXT, "safe-two") + .run(() -> { + String inner = USER_CONTEXT.get(); + sink(inner); // False Positive: currently flags (model limitation + }); + sink(outer); // should flag: tainted data reaches sink + }); + } + + public static void sink(String s) { + } +} \ No newline at end of file diff --git a/java/ql/test/library-tests/dataflow/scoped-values/options b/java/ql/test/library-tests/dataflow/scoped-values/options new file mode 100644 index 00000000000..c793109355a --- /dev/null +++ b/java/ql/test/library-tests/dataflow/scoped-values/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -source 25 -target 25 --enable-preview \ No newline at end of file diff --git a/java/ql/test/library-tests/dataflow/scoped-values/test.expected b/java/ql/test/library-tests/dataflow/scoped-values/test.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/java/ql/test/library-tests/dataflow/scoped-values/test.ql b/java/ql/test/library-tests/dataflow/scoped-values/test.ql new file mode 100644 index 00000000000..43534d2d3d8 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/scoped-values/test.ql @@ -0,0 +1,24 @@ +import java +import semmle.code.java.dataflow.TaintTracking + +module Config implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node n) { + exists(ArrayAccess aa | + aa.getArray().(VarAccess).getVariable().hasName("args") and + n.asExpr() = aa + ) + } + + predicate isSink(DataFlow::Node n) { + exists(MethodCall ma | + ma.getMethod().hasName("sink") and + n.asExpr() = ma.getAnArgument() + ) + } +} + +module Flow = TaintTracking::Global; + +from DataFlow::Node src, DataFlow::Node sink +where Flow::flow(src, sink) +select src, sink From 9f1e60ca6d7e16516a855d44de548e7aff166f82 Mon Sep 17 00:00:00 2001 From: idrissrio Date: Mon, 1 Sep 2025 17:41:02 +0200 Subject: [PATCH 2/6] Java: Add MaDs for `java.lang.scoped` --- java/ql/lib/ext/java.lang.scoped.model.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 java/ql/lib/ext/java.lang.scoped.model.yml diff --git a/java/ql/lib/ext/java.lang.scoped.model.yml b/java/ql/lib/ext/java.lang.scoped.model.yml new file mode 100644 index 00000000000..0b2c3af5a22 --- /dev/null +++ b/java/ql/lib/ext/java.lang.scoped.model.yml @@ -0,0 +1,19 @@ +extensions: + - addsTo: + pack: codeql/java-all + extensible: summaryModel + data: + - ["java.lang", "ScopedValue", False, "where", "(ScopedValue,Object)", "", "Argument[1]", "Argument[0].SyntheticField[java.lang.ScopedValue.boundValue]", "value", "manual"] + - ["java.lang", "ScopedValue", True, "get", "()", "", "Argument[this].SyntheticField[java.lang.ScopedValue.boundValue]", "ReturnValue", "value", "manual"] + - ["java.lang", "ScopedValue", False, "where", "(ScopedValue,Object)", "", "Argument[0]", "ReturnValue", "taint", "manual"] + - ["java.lang", "ScopedValue$Carrier", False, "where", "(ScopedValue,Object)", "", "Argument[1]", "Argument[0].SyntheticField[java.lang.ScopedValue.boundValue]", "value", "manual"] + - ["java.lang", "ScopedValue$Carrier", False, "where", "(ScopedValue,Object)", "", "Argument[0]", "ReturnValue", "taint", "manual"] + + - addsTo: + pack: codeql/java-all + extensible: neutralModel + data: + - ["java.lang", "ScopedValue", "newInstance", "()", "summary", "manual"] + - ["java.lang", "ScopedValue", "isBound", "()", "summary", "manual"] + - ["java.lang", "ScopedValue$Carrier", "run", "(Runnable)", "summary", "manual"] + - ["java.lang", "ScopedValue$Carrier", "call", "(Callable)", "summary", "manual"] \ No newline at end of file From a8541b9f7633ffef969cb564063887d93518e07c Mon Sep 17 00:00:00 2001 From: idrissrio Date: Mon, 1 Sep 2025 17:41:58 +0200 Subject: [PATCH 3/6] Java: accept new test results --- .../test/library-tests/dataflow/scoped-values/test.expected | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/java/ql/test/library-tests/dataflow/scoped-values/test.expected b/java/ql/test/library-tests/dataflow/scoped-values/test.expected index e69de29bb2d..5878a993141 100644 --- a/java/ql/test/library-tests/dataflow/scoped-values/test.expected +++ b/java/ql/test/library-tests/dataflow/scoped-values/test.expected @@ -0,0 +1,4 @@ +| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:12:22:12:26 | value | +| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:21:22:21:25 | user | +| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:31:30:31:34 | inner | +| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:33:22:33:26 | outer | From 2f4c728bb9be8aebb6878792e2ab13300cab595e Mon Sep 17 00:00:00 2001 From: idrissrio Date: Tue, 2 Sep 2025 13:28:44 +0200 Subject: [PATCH 4/6] Java: Add new change note --- java/ql/lib/change-notes/2025-09-02-scoped-values.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/lib/change-notes/2025-09-02-scoped-values.md diff --git a/java/ql/lib/change-notes/2025-09-02-scoped-values.md b/java/ql/lib/change-notes/2025-09-02-scoped-values.md new file mode 100644 index 00000000000..8758d1268f3 --- /dev/null +++ b/java/ql/lib/change-notes/2025-09-02-scoped-values.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added taint flow model for `java.lang.ScopedValue`. \ No newline at end of file From 117c41bd55293e8f4caa6c2c62675353ec9f2929 Mon Sep 17 00:00:00 2001 From: idrissrio Date: Fri, 5 Sep 2025 12:44:10 +0200 Subject: [PATCH 5/6] Java: Address review comment. Fix dataflow model --- java/ql/lib/ext/java.lang.scoped.model.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/java/ql/lib/ext/java.lang.scoped.model.yml b/java/ql/lib/ext/java.lang.scoped.model.yml index 0b2c3af5a22..3eeae5a5b1f 100644 --- a/java/ql/lib/ext/java.lang.scoped.model.yml +++ b/java/ql/lib/ext/java.lang.scoped.model.yml @@ -8,12 +8,12 @@ extensions: - ["java.lang", "ScopedValue", False, "where", "(ScopedValue,Object)", "", "Argument[0]", "ReturnValue", "taint", "manual"] - ["java.lang", "ScopedValue$Carrier", False, "where", "(ScopedValue,Object)", "", "Argument[1]", "Argument[0].SyntheticField[java.lang.ScopedValue.boundValue]", "value", "manual"] - ["java.lang", "ScopedValue$Carrier", False, "where", "(ScopedValue,Object)", "", "Argument[0]", "ReturnValue", "taint", "manual"] + - ["java.lang", "ScopedValue$Carrier", False, "run", "(Runnable)", "", "Argument[this]", "ReturnValue", "taint", "manual"] + - ["java.lang", "ScopedValue$Carrier", False, "call", "(Callable)", "", "Argument[this]", "ReturnValue", "taint", "manual"] - addsTo: pack: codeql/java-all extensible: neutralModel data: - ["java.lang", "ScopedValue", "newInstance", "()", "summary", "manual"] - - ["java.lang", "ScopedValue", "isBound", "()", "summary", "manual"] - - ["java.lang", "ScopedValue$Carrier", "run", "(Runnable)", "summary", "manual"] - - ["java.lang", "ScopedValue$Carrier", "call", "(Callable)", "summary", "manual"] \ No newline at end of file + - ["java.lang", "ScopedValue", "isBound", "()", "summary", "manual"] \ No newline at end of file From 666678a58214b6b9b441a80e2efc73dd43391739 Mon Sep 17 00:00:00 2001 From: idrissrio Date: Fri, 5 Sep 2025 12:44:42 +0200 Subject: [PATCH 6/6] Java: Address review comment. Inline dataflow annotation --- .../scoped-values/ScopedValueFlowTest.java | 23 ++++---- .../dataflow/scoped-values/test.expected | 52 +++++++++++++++++-- .../dataflow/scoped-values/test.ql | 26 ++-------- 3 files changed, 65 insertions(+), 36 deletions(-) diff --git a/java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java b/java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java index d71233f7dc8..2df07ae2f32 100644 --- a/java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java +++ b/java/ql/test/library-tests/dataflow/scoped-values/ScopedValueFlowTest.java @@ -1,15 +1,23 @@ +import java.lang.ScopedValue; + public class ScopedValueFlowTest { private static final ScopedValue USER_CONTEXT = ScopedValue.newInstance(); private static final ScopedValue SESSION_ID = ScopedValue.newInstance(); + public static String source(String label) { + return "tainted"; + } + + public static void sink(String value) {} + public static void main(String[] args) { - String userInput = args[0]; // source + String userInput = source(""); // Test 1: Basic scoped value binding and retrieval ScopedValue.where(USER_CONTEXT, userInput) .run(() -> { String value = USER_CONTEXT.get(); - sink(value); // should flag: tainted data reaches sink + sink(value); // $ hasTaintFlow }); // Test 2: Multiple scoped value bindings with chaining @@ -18,8 +26,8 @@ public class ScopedValueFlowTest { .run(() -> { String user = USER_CONTEXT.get(); String session = SESSION_ID.get(); - sink(user); // should flag: tainted data reaches sink - sink(session); // should NOT flag + sink(user); // $ hasTaintFlow + sink(session); // safe - should NOT have taint flow }); ScopedValue.where(USER_CONTEXT, userInput) @@ -28,12 +36,9 @@ public class ScopedValueFlowTest { ScopedValue.where(USER_CONTEXT, "safe-two") .run(() -> { String inner = USER_CONTEXT.get(); - sink(inner); // False Positive: currently flags (model limitation + sink(inner); // $ SPURIOUS: hasTaintFlow }); - sink(outer); // should flag: tainted data reaches sink + sink(outer); // $ hasTaintFlow }); } - - public static void sink(String s) { - } } \ No newline at end of file diff --git a/java/ql/test/library-tests/dataflow/scoped-values/test.expected b/java/ql/test/library-tests/dataflow/scoped-values/test.expected index 5878a993141..a617a94188e 100644 --- a/java/ql/test/library-tests/dataflow/scoped-values/test.expected +++ b/java/ql/test/library-tests/dataflow/scoped-values/test.expected @@ -1,4 +1,48 @@ -| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:12:22:12:26 | value | -| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:21:22:21:25 | user | -| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:31:30:31:34 | inner | -| ScopedValueFlowTest.java:6:28:6:34 | ...[...] | ScopedValueFlowTest.java:33:22:33:26 | outer | +models +| 1 | Summary: java.lang; ScopedValue; false; where; (ScopedValue,Object); ; Argument[1]; Argument[0].SyntheticField[java.lang.ScopedValue.boundValue]; value; manual | +| 2 | Summary: java.lang; ScopedValue; true; get; (); ; Argument[this].SyntheticField[java.lang.ScopedValue.boundValue]; ReturnValue; value; manual | +edges +| 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 | | +| 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 | | +| 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 | | +| 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 | | +| ScopedValueFlowTest.java:14:28:14:37 | source(...) : String | ScopedValueFlowTest.java:17:41:17:49 | userInput : String | provenance | | +| ScopedValueFlowTest.java:14:28:14:37 | source(...) : String | ScopedValueFlowTest.java:24:41:24:49 | userInput : String | provenance | | +| ScopedValueFlowTest.java:14:28:14:37 | source(...) : String | ScopedValueFlowTest.java:33:41:33:49 | userInput : String | provenance | | +| 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 | | +| 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 | +| 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 | +| ScopedValueFlowTest.java:19:32:19:49 | get(...) : String | ScopedValueFlowTest.java:20:22:20:26 | value | provenance | | +| 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 | | +| 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 | +| 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 | +| ScopedValueFlowTest.java:27:31:27:48 | get(...) : String | ScopedValueFlowTest.java:29:22:29:25 | user | provenance | | +| 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 | | +| 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 | +| 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 | +| ScopedValueFlowTest.java:35:32:35:49 | get(...) : String | ScopedValueFlowTest.java:41:22:41:26 | outer | provenance | | +| 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 | +| ScopedValueFlowTest.java:38:40:38:57 | get(...) : String | ScopedValueFlowTest.java:39:30:39:34 | inner | provenance | | +nodes +| ScopedValueFlowTest.java:4:46:4:57 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | +| ScopedValueFlowTest.java:14:28:14:37 | source(...) : String | semmle.label | source(...) : String | +| 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 | +| ScopedValueFlowTest.java:17:41:17:49 | userInput : String | semmle.label | userInput : String | +| ScopedValueFlowTest.java:19:32:19:43 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | +| ScopedValueFlowTest.java:19:32:19:49 | get(...) : String | semmle.label | get(...) : String | +| ScopedValueFlowTest.java:20:22:20:26 | value | semmle.label | value | +| 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 | +| ScopedValueFlowTest.java:24:41:24:49 | userInput : String | semmle.label | userInput : String | +| ScopedValueFlowTest.java:27:31:27:42 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | +| ScopedValueFlowTest.java:27:31:27:48 | get(...) : String | semmle.label | get(...) : String | +| ScopedValueFlowTest.java:29:22:29:25 | user | semmle.label | user | +| 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 | +| ScopedValueFlowTest.java:33:41:33:49 | userInput : String | semmle.label | userInput : String | +| ScopedValueFlowTest.java:35:32:35:43 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | +| ScopedValueFlowTest.java:35:32:35:49 | get(...) : String | semmle.label | get(...) : String | +| ScopedValueFlowTest.java:38:40:38:51 | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | semmle.label | USER_CONTEXT : ScopedValue [java.lang.ScopedValue.boundValue] : String | +| ScopedValueFlowTest.java:38:40:38:57 | get(...) : String | semmle.label | get(...) : String | +| ScopedValueFlowTest.java:39:30:39:34 | inner | semmle.label | inner | +| ScopedValueFlowTest.java:41:22:41:26 | outer | semmle.label | outer | +subpaths +testFailures diff --git a/java/ql/test/library-tests/dataflow/scoped-values/test.ql b/java/ql/test/library-tests/dataflow/scoped-values/test.ql index 43534d2d3d8..14dfc550b73 100644 --- a/java/ql/test/library-tests/dataflow/scoped-values/test.ql +++ b/java/ql/test/library-tests/dataflow/scoped-values/test.ql @@ -1,24 +1,4 @@ import java -import semmle.code.java.dataflow.TaintTracking - -module Config implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node n) { - exists(ArrayAccess aa | - aa.getArray().(VarAccess).getVariable().hasName("args") and - n.asExpr() = aa - ) - } - - predicate isSink(DataFlow::Node n) { - exists(MethodCall ma | - ma.getMethod().hasName("sink") and - n.asExpr() = ma.getAnArgument() - ) - } -} - -module Flow = TaintTracking::Global; - -from DataFlow::Node src, DataFlow::Node sink -where Flow::flow(src, sink) -select src, sink +import utils.test.InlineFlowTest +import TaintFlowTest +import TaintFlow::PathGraph