Skip to content

Commit 97128b1

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request github#3829 from asger-semmle/js/xss-substr
Approved by erik-krogh
2 parents d01904d + 03c91a6 commit 97128b1

File tree

4 files changed

+107
-4
lines changed

4 files changed

+107
-4
lines changed

‎javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,17 @@ module DomBasedXss {
2828
guard instanceof SanitizerGuard
2929
}
3030

31+
override predicate isAdditionalStoreStep(
32+
DataFlow::Node pred, DataFlow::SourceNode succ, string prop
33+
) {
34+
exists(DataFlow::PropRead read |
35+
pred = read.getBase() and
36+
succ = read and
37+
read.getPropertyName() = "hash" and
38+
prop = urlSuffixPseudoProperty()
39+
)
40+
}
41+
3142
override predicate isAdditionalLoadStoreStep(
3243
DataFlow::Node pred, DataFlow::Node succ, string predProp, string succProp
3344
) {
@@ -41,15 +52,29 @@ module DomBasedXss {
4152
}
4253

4354
override predicate isAdditionalLoadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
44-
exists(DataFlow::MethodCallNode call, string name |
45-
name = "substr" or name = "substring" or name = "slice"
46-
|
47-
call.getMethodName() = name and
55+
exists(DataFlow::MethodCallNode call |
56+
call.getMethodName() = ["substr", "substring", "slice"] and
4857
not call.getArgument(0).getIntValue() = 0 and
4958
pred = call.getReceiver() and
5059
succ = call and
5160
prop = urlSuffixPseudoProperty()
5261
)
62+
or
63+
exists(DataFlow::MethodCallNode call |
64+
call.getMethodName() = "exec" and pred = call.getArgument(0)
65+
or
66+
call.getMethodName() = "match" and pred = call.getReceiver()
67+
|
68+
succ = call and
69+
prop = urlSuffixPseudoProperty()
70+
)
71+
or
72+
exists(StringSplitCall split |
73+
split.getSeparator() = ["#", "?"] and
74+
pred = split.getBaseString() and
75+
succ = split.getASubstringRead(1) and
76+
prop = urlSuffixPseudoProperty()
77+
)
5378
}
5479

5580
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,23 @@ nodes
453453
| tst.js:414:19:414:31 | target.taint8 |
454454
| tst.js:415:18:415:30 | target.taint8 |
455455
| tst.js:415:18:415:30 | target.taint8 |
456+
| tst.js:422:7:422:46 | payload |
457+
| tst.js:422:17:422:31 | window.location |
458+
| tst.js:422:17:422:31 | window.location |
459+
| tst.js:422:17:422:46 | window. ... bstr(1) |
460+
| tst.js:423:18:423:24 | payload |
461+
| tst.js:423:18:423:24 | payload |
462+
| tst.js:425:7:425:55 | match |
463+
| tst.js:425:15:425:29 | window.location |
464+
| tst.js:425:15:425:29 | window.location |
465+
| tst.js:425:15:425:55 | window. ... (\\w+)/) |
466+
| tst.js:427:20:427:24 | match |
467+
| tst.js:427:20:427:27 | match[1] |
468+
| tst.js:427:20:427:27 | match[1] |
469+
| tst.js:430:18:430:32 | window.location |
470+
| tst.js:430:18:430:32 | window.location |
471+
| tst.js:430:18:430:51 | window. ... '#')[1] |
472+
| tst.js:430:18:430:51 | window. ... '#')[1] |
456473
| typeahead.js:20:13:20:45 | target |
457474
| typeahead.js:20:22:20:38 | document.location |
458475
| typeahead.js:20:22:20:38 | document.location |
@@ -882,6 +899,21 @@ edges
882899
| tst.js:414:19:414:31 | target.taint8 | tst.js:414:19:414:31 | target.taint8 |
883900
| tst.js:414:19:414:31 | target.taint8 | tst.js:415:18:415:30 | target.taint8 |
884901
| tst.js:414:19:414:31 | target.taint8 | tst.js:415:18:415:30 | target.taint8 |
902+
| tst.js:422:7:422:46 | payload | tst.js:423:18:423:24 | payload |
903+
| tst.js:422:7:422:46 | payload | tst.js:423:18:423:24 | payload |
904+
| tst.js:422:17:422:31 | window.location | tst.js:422:17:422:46 | window. ... bstr(1) |
905+
| tst.js:422:17:422:31 | window.location | tst.js:422:17:422:46 | window. ... bstr(1) |
906+
| tst.js:422:17:422:46 | window. ... bstr(1) | tst.js:422:7:422:46 | payload |
907+
| tst.js:425:7:425:55 | match | tst.js:427:20:427:24 | match |
908+
| tst.js:425:15:425:29 | window.location | tst.js:425:15:425:55 | window. ... (\\w+)/) |
909+
| tst.js:425:15:425:29 | window.location | tst.js:425:15:425:55 | window. ... (\\w+)/) |
910+
| tst.js:425:15:425:55 | window. ... (\\w+)/) | tst.js:425:7:425:55 | match |
911+
| tst.js:427:20:427:24 | match | tst.js:427:20:427:27 | match[1] |
912+
| tst.js:427:20:427:24 | match | tst.js:427:20:427:27 | match[1] |
913+
| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
914+
| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
915+
| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
916+
| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
885917
| typeahead.js:20:13:20:45 | target | typeahead.js:21:12:21:17 | target |
886918
| typeahead.js:20:22:20:38 | document.location | typeahead.js:20:22:20:45 | documen ... .search |
887919
| typeahead.js:20:22:20:38 | document.location | typeahead.js:20:22:20:45 | documen ... .search |
@@ -1009,6 +1041,9 @@ edges
10091041
| tst.js:403:18:403:30 | target.taint5 | tst.js:387:16:387:32 | document.location | tst.js:403:18:403:30 | target.taint5 | Cross-site scripting vulnerability due to $@. | tst.js:387:16:387:32 | document.location | user-provided value |
10101042
| tst.js:412:18:412:30 | target.taint7 | tst.js:387:16:387:32 | document.location | tst.js:412:18:412:30 | target.taint7 | Cross-site scripting vulnerability due to $@. | tst.js:387:16:387:32 | document.location | user-provided value |
10111043
| tst.js:415:18:415:30 | target.taint8 | tst.js:387:16:387:32 | document.location | tst.js:415:18:415:30 | target.taint8 | Cross-site scripting vulnerability due to $@. | tst.js:387:16:387:32 | document.location | user-provided value |
1044+
| tst.js:423:18:423:24 | payload | tst.js:422:17:422:31 | window.location | tst.js:423:18:423:24 | payload | Cross-site scripting vulnerability due to $@. | tst.js:422:17:422:31 | window.location | user-provided value |
1045+
| tst.js:427:20:427:27 | match[1] | tst.js:425:15:425:29 | window.location | tst.js:427:20:427:27 | match[1] | Cross-site scripting vulnerability due to $@. | tst.js:425:15:425:29 | window.location | user-provided value |
1046+
| tst.js:430:18:430:51 | window. ... '#')[1] | tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] | Cross-site scripting vulnerability due to $@. | tst.js:430:18:430:32 | window.location | user-provided value |
10121047
| typeahead.js:25:18:25:20 | val | typeahead.js:20:22:20:38 | document.location | typeahead.js:25:18:25:20 | val | Cross-site scripting vulnerability due to $@. | typeahead.js:20:22:20:38 | document.location | user-provided value |
10131048
| v-html.vue:2:8:2:23 | v-html=tainted | v-html.vue:6:42:6:58 | document.location | v-html.vue:2:8:2:23 | v-html=tainted | Cross-site scripting vulnerability due to $@. | v-html.vue:6:42:6:58 | document.location | user-provided value |
10141049
| 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 |

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,23 @@ nodes
453453
| tst.js:414:19:414:31 | target.taint8 |
454454
| tst.js:415:18:415:30 | target.taint8 |
455455
| tst.js:415:18:415:30 | target.taint8 |
456+
| tst.js:422:7:422:46 | payload |
457+
| tst.js:422:17:422:31 | window.location |
458+
| tst.js:422:17:422:31 | window.location |
459+
| tst.js:422:17:422:46 | window. ... bstr(1) |
460+
| tst.js:423:18:423:24 | payload |
461+
| tst.js:423:18:423:24 | payload |
462+
| tst.js:425:7:425:55 | match |
463+
| tst.js:425:15:425:29 | window.location |
464+
| tst.js:425:15:425:29 | window.location |
465+
| tst.js:425:15:425:55 | window. ... (\\w+)/) |
466+
| tst.js:427:20:427:24 | match |
467+
| tst.js:427:20:427:27 | match[1] |
468+
| tst.js:427:20:427:27 | match[1] |
469+
| tst.js:430:18:430:32 | window.location |
470+
| tst.js:430:18:430:32 | window.location |
471+
| tst.js:430:18:430:51 | window. ... '#')[1] |
472+
| tst.js:430:18:430:51 | window. ... '#')[1] |
456473
| typeahead.js:9:28:9:30 | loc |
457474
| typeahead.js:9:28:9:30 | loc |
458475
| typeahead.js:10:16:10:18 | loc |
@@ -886,6 +903,21 @@ edges
886903
| tst.js:414:19:414:31 | target.taint8 | tst.js:414:19:414:31 | target.taint8 |
887904
| tst.js:414:19:414:31 | target.taint8 | tst.js:415:18:415:30 | target.taint8 |
888905
| tst.js:414:19:414:31 | target.taint8 | tst.js:415:18:415:30 | target.taint8 |
906+
| tst.js:422:7:422:46 | payload | tst.js:423:18:423:24 | payload |
907+
| tst.js:422:7:422:46 | payload | tst.js:423:18:423:24 | payload |
908+
| tst.js:422:17:422:31 | window.location | tst.js:422:17:422:46 | window. ... bstr(1) |
909+
| tst.js:422:17:422:31 | window.location | tst.js:422:17:422:46 | window. ... bstr(1) |
910+
| tst.js:422:17:422:46 | window. ... bstr(1) | tst.js:422:7:422:46 | payload |
911+
| tst.js:425:7:425:55 | match | tst.js:427:20:427:24 | match |
912+
| tst.js:425:15:425:29 | window.location | tst.js:425:15:425:55 | window. ... (\\w+)/) |
913+
| tst.js:425:15:425:29 | window.location | tst.js:425:15:425:55 | window. ... (\\w+)/) |
914+
| tst.js:425:15:425:55 | window. ... (\\w+)/) | tst.js:425:7:425:55 | match |
915+
| tst.js:427:20:427:24 | match | tst.js:427:20:427:27 | match[1] |
916+
| tst.js:427:20:427:24 | match | tst.js:427:20:427:27 | match[1] |
917+
| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
918+
| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
919+
| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
920+
| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
889921
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |
890922
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |
891923
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,3 +418,14 @@ function test() {
418418
$('myId').html(target.taint9); // OK
419419
}
420420

421+
function hash2() {
422+
var payload = window.location.hash.substr(1);
423+
document.write(payload); // NOT OK
424+
425+
let match = window.location.hash.match(/hello (\w+)/);
426+
if (match) {
427+
document.write(match[1]); // NOT OK
428+
}
429+
430+
document.write(window.location.hash.split('#')[1]); // NOT OK
431+
}

0 commit comments

Comments
 (0)