Skip to content

Commit 86ee58d

Browse files
author
Max Schaefer
committed
JavaScript: Address review comments.
1 parent a8a8754 commit 86ee58d

File tree

2 files changed

+8
-14
lines changed

2 files changed

+8
-14
lines changed

javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ class PathSummary extends TPathSummary {
282282
result = hasCall
283283
}
284284

285-
/** Gets the flow label describing data at the end of this flow path. */
285+
/** Gets the flow label describing the value at the end of this flow path. */
286286
FlowLabel getEndLabel() {
287287
result = end
288288
}

javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,6 @@ module ClientSideUrlRedirect {
4444
}
4545

4646
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) {
47-
source instanceof RemoteFlowSource and
48-
lbl = DataFlow::FlowLabel::taint()
49-
or
5047
isDocumentURL(source.asExpr()) and
5148
lbl instanceof DocumentUrl
5249
}
@@ -55,11 +52,6 @@ module ClientSideUrlRedirect {
5552
sink instanceof Sink
5653
}
5754

58-
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel f) {
59-
sink instanceof UrlSink and
60-
f = DataFlow::FlowLabel::taint()
61-
}
62-
6355
override predicate isSanitizer(DataFlow::Node node) {
6456
super.isSanitizer(node) or
6557
node instanceof Sanitizer
@@ -76,6 +68,11 @@ module ClientSideUrlRedirect {
7668
}
7769
}
7870

71+
/** A source of remote user input, considered as a flow source for unvalidated URL redirects. */
72+
class RemoteFlowSourceAsSource extends Source {
73+
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
74+
}
75+
7976
/**
8077
* Holds if `queryAccess` is an expression that may access the query string
8178
* of a URL that flows into `nd` (that is, the part after the `?`).
@@ -105,13 +102,10 @@ module ClientSideUrlRedirect {
105102
)
106103
}
107104

108-
abstract class UrlSink extends DataFlow::Node {
109-
}
110-
111105
/**
112106
* A sink which is used to set the window location.
113107
*/
114-
class LocationSink extends UrlSink, DataFlow::ValueNode {
108+
class LocationSink extends Sink, DataFlow::ValueNode {
115109
LocationSink() {
116110
// A call to a `window.navigate` or `window.open`
117111
exists (string name |
@@ -152,7 +146,7 @@ module ClientSideUrlRedirect {
152146
/**
153147
* An expression that may be interpreted as the URL of a script.
154148
*/
155-
abstract class ScriptUrlSink extends UrlSink {
149+
abstract class ScriptUrlSink extends Sink {
156150
}
157151

158152
/**

0 commit comments

Comments
 (0)