Skip to content

Commit 67e6a4c

Browse files
committed
add a isXSSSink predicate to the client-side-url-redirection sinks
1 parent fc79242 commit 67e6a4c

File tree

1 file changed

+26
-6
lines changed

1 file changed

+26
-6
lines changed

javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@ module ClientSideUrlRedirect {
1919
/**
2020
* A data flow sink for unvalidated URL redirect vulnerabilities.
2121
*/
22-
abstract class Sink extends DataFlow::Node { }
22+
abstract class Sink extends DataFlow::Node {
23+
/** Holds if the sink can execute JavaScript code in the current context. */
24+
predicate isXSSSink() {
25+
none() // overwritten in subclasses
26+
}
27+
}
2328

2429
/**
2530
* A sanitizer for unvalidated URL redirect vulnerabilities.
@@ -84,37 +89,46 @@ module ClientSideUrlRedirect {
8489
* A sink which is used to set the window location.
8590
*/
8691
class LocationSink extends Sink, DataFlow::ValueNode {
92+
boolean xss;
93+
8794
LocationSink() {
8895
// A call to a `window.navigate` or `window.open`
8996
exists(string name | name = ["navigate", "open", "openDialog", "showModalDialog"] |
9097
this = DataFlow::globalVarRef(name).getACall().getArgument(0)
91-
)
98+
) and
99+
xss = false
92100
or
93101
// A call to `location.replace` or `location.assign`
94102
exists(DataFlow::MethodCallNode locationCall, string name |
95103
locationCall = DOM::locationRef().getAMethodCall(name) and
96104
this = locationCall.getArgument(0)
97105
|
98106
name = ["replace", "assign"]
99-
)
107+
) and
108+
xss = true
100109
or
101110
// An assignment to `location`
102-
exists(Assignment assgn | isLocation(assgn.getTarget()) and astNode = assgn.getRhs())
111+
exists(Assignment assgn | isLocation(assgn.getTarget()) and astNode = assgn.getRhs()) and
112+
xss = true
103113
or
104114
// An assignment to `location.href`, `location.protocol` or `location.hostname`
105115
exists(DataFlow::PropWrite pw, string propName |
106116
pw = DOM::locationRef().getAPropertyWrite(propName) and
107117
this = pw.getRhs()
108118
|
109-
propName = ["href", "protocol", "hostname"]
119+
propName = ["href", "protocol", "hostname"] and
120+
(if propName = "href" then xss = true else xss = false)
110121
)
111122
or
112123
// A redirection using the AngularJS `$location` service
113124
exists(AngularJS::ServiceReference service |
114125
service.getName() = "$location" and
115126
this.asExpr() = service.getAMethodCall("url").getArgument(0)
116-
)
127+
) and
128+
xss = false
117129
}
130+
131+
override predicate isXSSSink() { xss = true }
118132
}
119133

120134
/**
@@ -164,6 +178,8 @@ module ClientSideUrlRedirect {
164178
this = attr.getValueNode()
165179
)
166180
}
181+
182+
override predicate isXSSSink() { any() }
167183
}
168184

169185
/**
@@ -177,6 +193,8 @@ module ClientSideUrlRedirect {
177193
this = DataFlow::valueNode(pw.getRhs())
178194
)
179195
}
196+
197+
override predicate isXSSSink() { any() }
180198
}
181199

182200
/**
@@ -193,6 +211,8 @@ module ClientSideUrlRedirect {
193211
this = attr.getValue().flow()
194212
)
195213
}
214+
215+
override predicate isXSSSink() { any() }
196216
}
197217

198218
/**

0 commit comments

Comments
 (0)