Skip to content

Commit 2fce626

Browse files
author
Max Schaefer
committed
JavaScript: Add Range.prototype.createContextualFragment as an XSS sink.
1 parent 9caa9c1 commit 2fce626

File tree

4 files changed

+27
-3
lines changed

4 files changed

+27
-3
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ private DataFlow::SourceNode domElementCreationOrQuery() {
3737
exists(string methodName |
3838
methodName = "createElement" or
3939
methodName = "createElementNS" or
40+
methodName = "createRange" or
4041
methodName = "getElementById" or
4142
methodName = "querySelector"
4243
|

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,20 @@ module DomBasedXss {
127127
}
128128

129129
/**
130-
* An expression whose value is interpreted as HTML by a DOMParser.
130+
* An expression whose value is interpreted as HTML.
131131
*/
132-
class DomParserSink extends Sink {
133-
DomParserSink() {
132+
class HtmlParserSink extends Sink {
133+
HtmlParserSink() {
134134
exists(DataFlow::GlobalVarRefNode domParser |
135135
domParser.getName() = "DOMParser" and
136136
this = domParser.getAnInstantiation().getAMethodCall("parseFromString").getArgument(0)
137137
)
138+
or
139+
exists(DataFlow::MethodCallNode ccf |
140+
isDomValue(ccf.getReceiver().asExpr()) and
141+
ccf.getMethodName() = "createContextualFragment" and
142+
this = ccf.getArgument(0)
143+
)
138144
}
139145
}
140146

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,9 @@ nodes
175175
| tst.js:272:16:272:32 | document.location |
176176
| tst.js:275:7:275:10 | loc3 |
177177
| tst.js:277:22:277:29 | location |
178+
| tst.js:282:9:282:29 | tainted |
179+
| tst.js:282:19:282:29 | window.name |
180+
| tst.js:285:59:285:65 | tainted |
178181
| winjs.js:2:7:2:53 | tainted |
179182
| winjs.js:2:17:2:33 | document.location |
180183
| winjs.js:2:17:2:40 | documen ... .search |
@@ -313,6 +316,8 @@ edges
313316
| tst.js:252:23:252:29 | tainted | tst.js:244:39:244:55 | props.propTainted |
314317
| tst.js:272:9:272:32 | loc3 | tst.js:275:7:275:10 | loc3 |
315318
| tst.js:272:16:272:32 | document.location | tst.js:272:9:272:32 | loc3 |
319+
| tst.js:282:9:282:29 | tainted | tst.js:285:59:285:65 | tainted |
320+
| tst.js:282:19:282:29 | window.name | tst.js:282:9:282:29 | tainted |
316321
| winjs.js:2:7:2:53 | tainted | winjs.js:3:43:3:49 | tainted |
317322
| winjs.js:2:7:2:53 | tainted | winjs.js:4:43:4:49 | tainted |
318323
| winjs.js:2:17:2:33 | document.location | winjs.js:2:17:2:40 | documen ... .search |
@@ -386,5 +391,8 @@ edges
386391
| 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 |
387392
| tst.js:275:7:275:10 | loc3 | tst.js:272:16:272:32 | document.location | tst.js:275:7:275:10 | loc3 | Cross-site scripting vulnerability due to $@. | tst.js:272:16:272:32 | document.location | user-provided value |
388393
| tst.js:277:22:277:29 | location | tst.js:277:22:277:29 | location | tst.js:277:22:277:29 | location | Cross-site scripting vulnerability due to $@. | tst.js:277:22:277:29 | location | user-provided value |
394+
| tst.js:285:59:285:65 | tainted | tst.js:282:9:282:29 | tainted | tst.js:285:59:285:65 | tainted | Cross-site scripting vulnerability due to $@. | tst.js:282:9:282:29 | tainted | user-provided value |
395+
| tst.js:285:59:285:65 | tainted | tst.js:282:19:282:29 | window.name | tst.js:285:59:285:65 | tainted | Cross-site scripting vulnerability due to $@. | tst.js:282:19:282:29 | window.name | user-provided value |
396+
| tst.js:285:59:285:65 | tainted | tst.js:285:59:285:65 | tainted | tst.js:285:59:285:65 | tainted | Cross-site scripting vulnerability due to $@. | tst.js:285:59:285:65 | tainted | user-provided value |
389397
| 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 |
390398
| 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: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,3 +276,12 @@ function jqueryLocation() {
276276

277277
$("body").append(location); // NOT OK
278278
}
279+
280+
281+
function testCreateContextualFragment() {
282+
var tainted = window.name;
283+
var range = document.createRange();
284+
range.selectNode(document.getElementsByTagName("div").item(0));
285+
var documentFragment = range.createContextualFragment(tainted); // NOT OK
286+
document.body.appendChild(documentFragment);
287+
}

0 commit comments

Comments
 (0)