Skip to content

Commit 26dcbf7

Browse files
committed
JS: Migrate URLSearchParams model to flow summaries
1 parent f531f44 commit 26dcbf7

File tree

4 files changed

+72
-9
lines changed

4 files changed

+72
-9
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
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 modelled 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+
}
Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
function u1() {
22
const searchParams = new URLSearchParams(source("u1.1"));
3-
sink(searchParams.get("x")); // $ MISSING: hasTaintFlow=u1.1
4-
sink(searchParams.get(unknown())); // $ MISSING: hasTaintFlow=u1.1
5-
sink(searchParams.getAll("x")); // $ MISSING: hasTaintFlow=u1.1
6-
sink(searchParams.getAll(unknown())); // $ MISSING: hasTaintFlow=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
77
}
88

99
function u2() {
1010
const url = new URL(source("u2.1"));
11-
sink(url.searchParams.get("x")); // $ MISSING: hasTaintFlow=u2.1
12-
sink(url.searchParams.get(unknown())); // $ MISSING: hasTaintFlow=u2.1
13-
sink(url.searchParams.getAll("x")); // $ MISSING: hasTaintFlow=u2.1
14-
sink(url.searchParams.getAll(unknown())); // $ MISSING: hasTaintFlow=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
1515
}

0 commit comments

Comments
 (0)