Skip to content

Commit df42707

Browse files
author
Max Schaefer
authored
Merge pull request #675 from asger-semmle/window.name
JS: Add window.name as remote flow source
2 parents 41a4807 + a96c53f commit df42707

File tree

4 files changed

+41
-1
lines changed

4 files changed

+41
-1
lines changed

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,4 +199,24 @@ private class PostMessageEventParameter extends RemoteFlowSource {
199199
override string getSourceType() {
200200
result = "postMessage event"
201201
}
202-
}
202+
}
203+
204+
/**
205+
* An access to `window.name`, which can be controlled by the opener of the window,
206+
* even if the window is opened from a foreign domain.
207+
*/
208+
private class WindowNameAccess extends RemoteFlowSource {
209+
WindowNameAccess() {
210+
this = DataFlow::globalObjectRef().getAPropertyRead("name")
211+
or
212+
// Reference to `name` on a container that does not assign to it.
213+
this.accessesGlobal("name") and
214+
not exists(VarDef def |
215+
def.getAVariable().(GlobalVariable).getName() = "name" and
216+
def.getContainer() = this.asExpr().getContainer())
217+
}
218+
219+
override string getSourceType() {
220+
result = "Window name"
221+
}
222+
}

javascript/ql/test/query-tests/Security/CWE-079/StoredXss.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ nodes
205205
| tst.js:244:39:244:55 | props.propTainted |
206206
| tst.js:248:60:248:82 | this.st ... Tainted |
207207
| tst.js:252:23:252:29 | tainted |
208+
| tst.js:256:7:256:17 | window.name |
209+
| tst.js:257:7:257:10 | name |
210+
| tst.js:261:11:261:21 | window.name |
208211
| winjs.js:2:7:2:53 | tainted |
209212
| winjs.js:2:17:2:33 | document.location |
210213
| winjs.js:2:17:2:40 | documen ... .search |

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@ nodes
162162
| tst.js:244:39:244:55 | props.propTainted |
163163
| tst.js:248:60:248:82 | this.st ... Tainted |
164164
| tst.js:252:23:252:29 | tainted |
165+
| tst.js:256:7:256:17 | window.name |
166+
| tst.js:257:7:257:10 | name |
167+
| tst.js:261:11:261:21 | window.name |
165168
| winjs.js:2:7:2:53 | tainted |
166169
| winjs.js:2:17:2:33 | document.location |
167170
| winjs.js:2:17:2:40 | documen ... .search |
@@ -360,5 +363,8 @@ edges
360363
| tst.js:224:28:224:46 | this.props.tainted3 | tst.js:194:19:194:35 | document.location | tst.js:224:28:224:46 | this.props.tainted3 | Cross-site scripting vulnerability due to $@. | tst.js:194:19:194:35 | document.location | user-provided value |
361364
| tst.js:228:32:228:49 | prevProps.tainted4 | tst.js:194:19:194:35 | document.location | tst.js:228:32:228:49 | prevProps.tainted4 | Cross-site scripting vulnerability due to $@. | tst.js:194:19:194:35 | document.location | user-provided value |
362365
| tst.js:248:60:248:82 | this.st ... Tainted | tst.js:194:19:194:35 | document.location | tst.js:248:60:248:82 | this.st ... Tainted | Cross-site scripting vulnerability due to $@. | tst.js:194:19:194:35 | document.location | user-provided value |
366+
| tst.js:256:7:256:17 | window.name | tst.js:256:7:256:17 | window.name | tst.js:256:7:256:17 | window.name | Cross-site scripting vulnerability due to $@. | tst.js:256:7:256:17 | window.name | user-provided value |
367+
| tst.js:257:7:257:10 | name | tst.js:257:7:257:10 | name | tst.js:257:7:257:10 | name | Cross-site scripting vulnerability due to $@. | tst.js:257:7:257:10 | name | user-provided value |
368+
| tst.js:261:11:261:21 | window.name | tst.js:261:11:261:21 | window.name | tst.js:261:11:261:21 | window.name | Cross-site scripting vulnerability due to $@. | tst.js:261:11:261:21 | window.name | user-provided value |
363369
| winjs.js:3:43:3:49 | tainted | winjs.js:2:17:2:33 | document.location | winjs.js:3:43:3:49 | tainted | Cross-site scripting vulnerability due to $@. | winjs.js:2:17:2:33 | document.location | user-provided value |
364370
| winjs.js:4:43:4:49 | tainted | winjs.js:2:17:2:33 | document.location | winjs.js:4:43:4:49 | tainted | Cross-site scripting vulnerability due to $@. | winjs.js:2:17:2:33 | document.location | user-provided value |

javascript/ql/test/query-tests/Security/CWE-079/tst.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,14 @@ function react(){
251251

252252
(<C3 propTainted={tainted}/>);
253253
}
254+
255+
function windowName() {
256+
$(window.name); // NOT OK
257+
$(name); // NOT OK
258+
}
259+
function windowNameAssigned() {
260+
for (name of ['a', 'b']) {
261+
$(window.name); // NOT OK
262+
$(name); // OK
263+
}
264+
}

0 commit comments

Comments
 (0)