Skip to content

Commit e02577d

Browse files
asgerfasgerf
authored
Merge pull request #18768 from asgerf/js/url-search-params
JS: Migrate model of URLSearchParams
2 parents 5f4871d + 7df3e64 commit e02577d

File tree

7 files changed

+180
-1
lines changed

7 files changed

+180
-1
lines changed

‎javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,7 @@ module TaintTracking {
656656
/**
657657
* A taint propagating data flow edge arising from URL parameter parsing.
658658
*/
659-
private class UrlSearchParamsTaintStep extends DataFlow::SharedFlowStep {
659+
private class UrlSearchParamsTaintStep extends DataFlow::LegacyFlowStep {
660660
/**
661661
* Holds if `succ` is a `URLSearchParams` providing access to the
662662
* parameters encoded in `pred`.

‎javascript/ql/lib/semmle/javascript/internal/flow_summaries/AllFlowSummaries.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ private import Promises
1111
private import Sets
1212
private import Strings
1313
private import DynamicImportStep
14+
private import UrlSearchParams

‎javascript/ql/lib/semmle/javascript/internal/flow_summaries/UrlSearchParams.qll‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Contains a summary for `URLSearchParams` and `URL` objects.
3+
*
4+
* For now, the `URLSearchParams` object is modeled as a `Map` object.
5+
*/
6+
7+
private import javascript
8+
9+
DataFlow::SourceNode urlConstructorRef() { result = DataFlow::globalVarRef("URL") }
10+
11+
DataFlow::SourceNode urlSearchParamsConstructorRef() {
12+
result = DataFlow::globalVarRef("URLSearchParams")
13+
}
14+
15+
class URLSearchParams extends DataFlow::SummarizedCallable {
16+
URLSearchParams() { this = "URLSearchParams" }
17+
18+
override DataFlow::InvokeNode getACallSimple() {
19+
result = urlSearchParamsConstructorRef().getAnInstantiation()
20+
}
21+
22+
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
23+
// Taint the MapKey and MapValue so methods named 'get' and 'forEach' etc can extract the taint.
24+
// Also taint the object itself since it has a tainted toString() value
25+
input = "Argument[0]" and
26+
output = ["ReturnValue", "ReturnValue.MapKey", "ReturnValue.MapValue"] and
27+
preservesValue = false
28+
}
29+
}
30+
31+
class GetAll extends DataFlow::SummarizedCallable {
32+
GetAll() { this = "getAll" }
33+
34+
override DataFlow::MethodCallNode getACallSimple() {
35+
result.getMethodName() = "getAll" and result.getNumArgument() = 1
36+
}
37+
38+
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
39+
input = "Argument[this].MapValue" and
40+
output = "ReturnValue.ArrayElement" and
41+
preservesValue = true
42+
}
43+
}
44+
45+
class URLConstructor extends DataFlow::SummarizedCallable {
46+
URLConstructor() { this = "URL" }
47+
48+
override DataFlow::InvokeNode getACallSimple() {
49+
result = urlConstructorRef().getAnInstantiation()
50+
}
51+
52+
override predicate propagatesFlow(string input, string output, boolean preservesValue) {
53+
input = "Argument[0]" and
54+
output =
55+
[
56+
"ReturnValue.Member[searchParams].MapKey",
57+
"ReturnValue.Member[searchParams].MapValue",
58+
"ReturnValue.Member[searchParams,hash,search]",
59+
] and
60+
preservesValue = false
61+
}
62+
}

‎javascript/ql/src/change-notes/2025-02-13-url-search-params.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: fix
3+
---
4+
* Fixed a recently-introduced bug that prevented taint tracking through `URLSearchParams` objects.
5+
The original behaviour has been restored and taint should once again be tracked through such objects.

‎javascript/ql/test/library-tests/TripleDot/urlsearchparams.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
function u1() {
2+
const searchParams = new URLSearchParams(source("u1.1"));
3+
sink(searchParams.get("x")); // $ hasTaintFlow=u1.1
4+
sink(searchParams.get(unknown())); // $ hasTaintFlow=u1.1
5+
sink(searchParams.getAll("x")); // $ hasTaintFlow=u1.1
6+
sink(searchParams.getAll(unknown())); // $ hasTaintFlow=u1.1
7+
}
8+
9+
function u2() {
10+
const url = new URL(source("u2.1"));
11+
sink(url.searchParams.get("x")); // $ hasTaintFlow=u2.1
12+
sink(url.searchParams.get(unknown())); // $ hasTaintFlow=u2.1
13+
sink(url.searchParams.getAll("x")); // $ hasTaintFlow=u2.1
14+
sink(url.searchParams.getAll(unknown())); // $ hasTaintFlow=u2.1
15+
}

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,15 @@ nodes
336336
| translate.js:6:7:6:39 | target | semmle.label | target |
337337
| translate.js:6:16:6:39 | documen ... .search | semmle.label | documen ... .search |
338338
| translate.js:7:7:7:61 | searchParams | semmle.label | searchParams |
339+
| translate.js:7:7:7:61 | searchParams [MapValue] | semmle.label | searchParams [MapValue] |
339340
| translate.js:7:22:7:61 | new URL ... ing(1)) | semmle.label | new URL ... ing(1)) |
341+
| translate.js:7:22:7:61 | new URL ... ing(1)) [MapValue] | semmle.label | new URL ... ing(1)) [MapValue] |
340342
| translate.js:7:42:7:47 | target | semmle.label | target |
341343
| translate.js:7:42:7:60 | target.substring(1) | semmle.label | target.substring(1) |
344+
| translate.js:7:42:7:60 | target.substring(1) | semmle.label | target.substring(1) |
345+
| translate.js:7:42:7:60 | target.substring(1) | semmle.label | target.substring(1) |
342346
| translate.js:9:27:9:38 | searchParams | semmle.label | searchParams |
347+
| translate.js:9:27:9:38 | searchParams [MapValue] | semmle.label | searchParams [MapValue] |
343348
| translate.js:9:27:9:50 | searchP ... 'term') | semmle.label | searchP ... 'term') |
344349
| trusted-types-lib.js:1:28:1:28 | x | semmle.label | x |
345350
| trusted-types-lib.js:2:12:2:12 | x | semmle.label | x |
@@ -371,17 +376,27 @@ nodes
371376
| tst.js:12:5:12:42 | '<div s ... 'px">' | semmle.label | '<div s ... 'px">' |
372377
| tst.js:12:28:12:33 | target | semmle.label | target |
373378
| tst.js:17:7:17:56 | params | semmle.label | params |
379+
| tst.js:17:7:17:56 | params [MapValue] | semmle.label | params [MapValue] |
380+
| tst.js:17:16:17:43 | (new UR ... ation)) [searchParams, MapValue] | semmle.label | (new UR ... ation)) [searchParams, MapValue] |
374381
| tst.js:17:16:17:43 | (new UR ... ation)) [searchParams] | semmle.label | (new UR ... ation)) [searchParams] |
375382
| tst.js:17:16:17:56 | (new UR ... hParams | semmle.label | (new UR ... hParams |
383+
| tst.js:17:16:17:56 | (new UR ... hParams [MapValue] | semmle.label | (new UR ... hParams [MapValue] |
384+
| tst.js:17:17:17:42 | new URL ... cation) [searchParams, MapValue] | semmle.label | new URL ... cation) [searchParams, MapValue] |
376385
| tst.js:17:17:17:42 | new URL ... cation) [searchParams] | semmle.label | new URL ... cation) [searchParams] |
377386
| tst.js:17:25:17:41 | document.location | semmle.label | document.location |
378387
| tst.js:18:18:18:23 | params | semmle.label | params |
388+
| tst.js:18:18:18:23 | params [MapValue] | semmle.label | params [MapValue] |
379389
| tst.js:18:18:18:35 | params.get('name') | semmle.label | params.get('name') |
380390
| tst.js:20:7:20:61 | searchParams | semmle.label | searchParams |
391+
| tst.js:20:7:20:61 | searchParams [MapValue] | semmle.label | searchParams [MapValue] |
381392
| tst.js:20:22:20:61 | new URL ... ing(1)) | semmle.label | new URL ... ing(1)) |
393+
| tst.js:20:22:20:61 | new URL ... ing(1)) [MapValue] | semmle.label | new URL ... ing(1)) [MapValue] |
382394
| tst.js:20:42:20:47 | target | semmle.label | target |
383395
| tst.js:20:42:20:60 | target.substring(1) | semmle.label | target.substring(1) |
396+
| tst.js:20:42:20:60 | target.substring(1) | semmle.label | target.substring(1) |
397+
| tst.js:20:42:20:60 | target.substring(1) | semmle.label | target.substring(1) |
384398
| tst.js:21:18:21:29 | searchParams | semmle.label | searchParams |
399+
| tst.js:21:18:21:29 | searchParams [MapValue] | semmle.label | searchParams [MapValue] |
385400
| tst.js:21:18:21:41 | searchP ... 'name') | semmle.label | searchP ... 'name') |
386401
| tst.js:24:14:24:19 | target | semmle.label | target |
387402
| tst.js:26:18:26:23 | target | semmle.label | target |
@@ -487,12 +502,17 @@ nodes
487502
| tst.js:310:10:310:10 | e | semmle.label | e |
488503
| tst.js:311:20:311:20 | e | semmle.label | e |
489504
| tst.js:316:35:316:42 | location | semmle.label | location |
505+
| tst.js:327:10:327:35 | new URL ... cation) [searchParams, MapValue] | semmle.label | new URL ... cation) [searchParams, MapValue] |
490506
| tst.js:327:10:327:35 | new URL ... cation) [searchParams] | semmle.label | new URL ... cation) [searchParams] |
491507
| tst.js:327:18:327:34 | document.location | semmle.label | document.location |
492508
| tst.js:331:7:331:43 | params | semmle.label | params |
509+
| tst.js:331:7:331:43 | params [MapValue] | semmle.label | params [MapValue] |
510+
| tst.js:331:16:331:30 | getTaintedUrl() [searchParams, MapValue] | semmle.label | getTaintedUrl() [searchParams, MapValue] |
493511
| tst.js:331:16:331:30 | getTaintedUrl() [searchParams] | semmle.label | getTaintedUrl() [searchParams] |
494512
| tst.js:331:16:331:43 | getTain ... hParams | semmle.label | getTain ... hParams |
513+
| tst.js:331:16:331:43 | getTain ... hParams [MapValue] | semmle.label | getTain ... hParams [MapValue] |
495514
| tst.js:332:18:332:23 | params | semmle.label | params |
515+
| tst.js:332:18:332:23 | params [MapValue] | semmle.label | params [MapValue] |
496516
| tst.js:332:18:332:35 | params.get('name') | semmle.label | params.get('name') |
497517
| tst.js:341:12:341:37 | new URL ... cation) [hash] | semmle.label | new URL ... cation) [hash] |
498518
| tst.js:341:20:341:36 | document.location | semmle.label | document.location |
@@ -900,10 +920,18 @@ edges
900920
| translate.js:6:7:6:39 | target | translate.js:7:42:7:47 | target | provenance | |
901921
| translate.js:6:16:6:39 | documen ... .search | translate.js:6:7:6:39 | target | provenance | |
902922
| translate.js:7:7:7:61 | searchParams | translate.js:9:27:9:38 | searchParams | provenance | |
923+
| translate.js:7:7:7:61 | searchParams [MapValue] | translate.js:9:27:9:38 | searchParams [MapValue] | provenance | |
903924
| translate.js:7:22:7:61 | new URL ... ing(1)) | translate.js:7:7:7:61 | searchParams | provenance | |
925+
| translate.js:7:22:7:61 | new URL ... ing(1)) [MapValue] | translate.js:7:7:7:61 | searchParams [MapValue] | provenance | |
904926
| translate.js:7:42:7:47 | target | translate.js:7:42:7:60 | target.substring(1) | provenance | |
927+
| translate.js:7:42:7:47 | target | translate.js:7:42:7:60 | target.substring(1) | provenance | Config |
928+
| translate.js:7:42:7:47 | target | translate.js:7:42:7:60 | target.substring(1) | provenance | Config |
905929
| translate.js:7:42:7:60 | target.substring(1) | translate.js:7:22:7:61 | new URL ... ing(1)) | provenance | |
930+
| translate.js:7:42:7:60 | target.substring(1) | translate.js:7:22:7:61 | new URL ... ing(1)) [MapValue] | provenance | |
931+
| translate.js:7:42:7:60 | target.substring(1) | translate.js:7:22:7:61 | new URL ... ing(1)) [MapValue] | provenance | |
932+
| translate.js:7:42:7:60 | target.substring(1) | translate.js:7:22:7:61 | new URL ... ing(1)) [MapValue] | provenance | |
906933
| translate.js:9:27:9:38 | searchParams | translate.js:9:27:9:50 | searchP ... 'term') | provenance | Config |
934+
| translate.js:9:27:9:38 | searchParams [MapValue] | translate.js:9:27:9:50 | searchP ... 'term') | provenance | |
907935
| trusted-types-lib.js:1:28:1:28 | x | trusted-types-lib.js:2:12:2:12 | x | provenance | |
908936
| trusted-types.js:3:62:3:62 | x | trusted-types.js:3:67:3:67 | x | provenance | |
909937
| trusted-types.js:4:20:4:30 | window.name | trusted-types.js:3:62:3:62 | x | provenance | |
@@ -932,16 +960,30 @@ edges
932960
| tst.js:8:37:8:114 | documen ... t=")+8) | tst.js:8:18:8:126 | "<OPTIO ... PTION>" | provenance | Config |
933961
| tst.js:12:28:12:33 | target | tst.js:12:5:12:42 | '<div s ... 'px">' | provenance | Config |
934962
| tst.js:17:7:17:56 | params | tst.js:18:18:18:23 | params | provenance | |
963+
| tst.js:17:7:17:56 | params [MapValue] | tst.js:18:18:18:23 | params [MapValue] | provenance | |
964+
| tst.js:17:16:17:43 | (new UR ... ation)) [searchParams, MapValue] | tst.js:17:16:17:56 | (new UR ... hParams [MapValue] | provenance | |
935965
| tst.js:17:16:17:43 | (new UR ... ation)) [searchParams] | tst.js:17:16:17:56 | (new UR ... hParams | provenance | |
936966
| tst.js:17:16:17:56 | (new UR ... hParams | tst.js:17:7:17:56 | params | provenance | |
967+
| tst.js:17:16:17:56 | (new UR ... hParams [MapValue] | tst.js:17:7:17:56 | params [MapValue] | provenance | |
968+
| tst.js:17:17:17:42 | new URL ... cation) [searchParams, MapValue] | tst.js:17:16:17:43 | (new UR ... ation)) [searchParams, MapValue] | provenance | |
937969
| tst.js:17:17:17:42 | new URL ... cation) [searchParams] | tst.js:17:16:17:43 | (new UR ... ation)) [searchParams] | provenance | |
970+
| tst.js:17:25:17:41 | document.location | tst.js:17:17:17:42 | new URL ... cation) [searchParams, MapValue] | provenance | |
938971
| tst.js:17:25:17:41 | document.location | tst.js:17:17:17:42 | new URL ... cation) [searchParams] | provenance | |
939972
| tst.js:18:18:18:23 | params | tst.js:18:18:18:35 | params.get('name') | provenance | Config |
973+
| tst.js:18:18:18:23 | params [MapValue] | tst.js:18:18:18:35 | params.get('name') | provenance | |
940974
| tst.js:20:7:20:61 | searchParams | tst.js:21:18:21:29 | searchParams | provenance | |
975+
| tst.js:20:7:20:61 | searchParams [MapValue] | tst.js:21:18:21:29 | searchParams [MapValue] | provenance | |
941976
| tst.js:20:22:20:61 | new URL ... ing(1)) | tst.js:20:7:20:61 | searchParams | provenance | |
977+
| tst.js:20:22:20:61 | new URL ... ing(1)) [MapValue] | tst.js:20:7:20:61 | searchParams [MapValue] | provenance | |
942978
| tst.js:20:42:20:47 | target | tst.js:20:42:20:60 | target.substring(1) | provenance | |
979+
| tst.js:20:42:20:47 | target | tst.js:20:42:20:60 | target.substring(1) | provenance | Config |
980+
| tst.js:20:42:20:47 | target | tst.js:20:42:20:60 | target.substring(1) | provenance | Config |
943981
| tst.js:20:42:20:60 | target.substring(1) | tst.js:20:22:20:61 | new URL ... ing(1)) | provenance | |
982+
| tst.js:20:42:20:60 | target.substring(1) | tst.js:20:22:20:61 | new URL ... ing(1)) [MapValue] | provenance | |
983+
| tst.js:20:42:20:60 | target.substring(1) | tst.js:20:22:20:61 | new URL ... ing(1)) [MapValue] | provenance | |
984+
| tst.js:20:42:20:60 | target.substring(1) | tst.js:20:22:20:61 | new URL ... ing(1)) [MapValue] | provenance | |
944985
| tst.js:21:18:21:29 | searchParams | tst.js:21:18:21:41 | searchP ... 'name') | provenance | Config |
986+
| tst.js:21:18:21:29 | searchParams [MapValue] | tst.js:21:18:21:41 | searchP ... 'name') | provenance | |
945987
| tst.js:24:14:24:19 | target | tst.js:26:18:26:23 | target | provenance | |
946988
| tst.js:28:5:28:28 | documen ... .search | tst.js:24:14:24:19 | target | provenance | |
947989
| tst.js:31:10:31:33 | documen ... .search | tst.js:34:16:34:20 | bar() | provenance | |
@@ -1033,12 +1075,18 @@ edges
10331075
| tst.js:302:10:302:10 | e | tst.js:303:20:303:20 | e | provenance | |
10341076
| tst.js:308:10:308:17 | location | tst.js:310:10:310:10 | e | provenance | |
10351077
| tst.js:310:10:310:10 | e | tst.js:311:20:311:20 | e | provenance | |
1078+
| tst.js:327:10:327:35 | new URL ... cation) [searchParams, MapValue] | tst.js:331:16:331:30 | getTaintedUrl() [searchParams, MapValue] | provenance | |
10361079
| tst.js:327:10:327:35 | new URL ... cation) [searchParams] | tst.js:331:16:331:30 | getTaintedUrl() [searchParams] | provenance | |
1080+
| tst.js:327:18:327:34 | document.location | tst.js:327:10:327:35 | new URL ... cation) [searchParams, MapValue] | provenance | |
10371081
| tst.js:327:18:327:34 | document.location | tst.js:327:10:327:35 | new URL ... cation) [searchParams] | provenance | |
10381082
| tst.js:331:7:331:43 | params | tst.js:332:18:332:23 | params | provenance | |
1083+
| tst.js:331:7:331:43 | params [MapValue] | tst.js:332:18:332:23 | params [MapValue] | provenance | |
1084+
| tst.js:331:16:331:30 | getTaintedUrl() [searchParams, MapValue] | tst.js:331:16:331:43 | getTain ... hParams [MapValue] | provenance | |
10391085
| tst.js:331:16:331:30 | getTaintedUrl() [searchParams] | tst.js:331:16:331:43 | getTain ... hParams | provenance | |
10401086
| tst.js:331:16:331:43 | getTain ... hParams | tst.js:331:7:331:43 | params | provenance | |
1087+
| tst.js:331:16:331:43 | getTain ... hParams [MapValue] | tst.js:331:7:331:43 | params [MapValue] | provenance | |
10411088
| tst.js:332:18:332:23 | params | tst.js:332:18:332:35 | params.get('name') | provenance | Config |
1089+
| tst.js:332:18:332:23 | params [MapValue] | tst.js:332:18:332:35 | params.get('name') | provenance | |
10421090
| tst.js:341:12:341:37 | new URL ... cation) [hash] | tst.js:343:5:343:12 | getUrl() [hash] | provenance | |
10431091
| tst.js:341:20:341:36 | document.location | tst.js:341:12:341:37 | new URL ... cation) [hash] | provenance | |
10441092
| tst.js:343:5:343:12 | getUrl() [hash] | tst.js:343:5:343:17 | getUrl().hash | provenance | |

0 commit comments

Comments
 (0)