Skip to content

Commit 40acb95

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#5397 from erik-krogh/globalSanitizer
Approved by asgerf
2 parents c08230c + 1dcfc38 commit 40acb95

File tree

4 files changed

+37
-1
lines changed

4 files changed

+37
-1
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,15 @@ module Shared {
2828
abstract class SanitizerGuard extends TaintTracking::SanitizerGuardNode { }
2929

3030
/**
31-
* A regexp replacement involving an HTML meta-character, viewed as a sanitizer for
31+
* A global regexp replacement involving an HTML meta-character, viewed as a sanitizer for
3232
* XSS vulnerabilities.
3333
*
3434
* The XSS queries do not attempt to reason about correctness or completeness of sanitizers,
3535
* so any such replacement stops taint propagation.
3636
*/
3737
class MetacharEscapeSanitizer extends Sanitizer, StringReplaceCall {
3838
MetacharEscapeSanitizer() {
39+
this.isGlobal() and
3940
exists(RegExpConstant c |
4041
c.getLiteral() = getRegExp().asExpr() and
4142
c.getValue().regexpMatch("['\"&<>]")

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,13 @@ nodes
653653
| tst.js:424:18:424:48 | window. ... it('#') |
654654
| tst.js:424:18:424:51 | window. ... '#')[1] |
655655
| tst.js:424:18:424:51 | window. ... '#')[1] |
656+
| tst.js:428:7:428:39 | target |
657+
| tst.js:428:16:428:32 | document.location |
658+
| tst.js:428:16:428:32 | document.location |
659+
| tst.js:428:16:428:39 | documen ... .search |
660+
| tst.js:430:18:430:23 | target |
661+
| tst.js:430:18:430:89 | target. ... data>') |
662+
| tst.js:430:18:430:89 | target. ... data>') |
656663
| typeahead.js:20:13:20:45 | target |
657664
| typeahead.js:20:22:20:38 | document.location |
658665
| typeahead.js:20:22:20:38 | document.location |
@@ -1283,6 +1290,12 @@ edges
12831290
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') |
12841291
| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] |
12851292
| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] |
1293+
| tst.js:428:7:428:39 | target | tst.js:430:18:430:23 | target |
1294+
| tst.js:428:16:428:32 | document.location | tst.js:428:16:428:39 | documen ... .search |
1295+
| tst.js:428:16:428:32 | document.location | tst.js:428:16:428:39 | documen ... .search |
1296+
| tst.js:428:16:428:39 | documen ... .search | tst.js:428:7:428:39 | target |
1297+
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') |
1298+
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') |
12861299
| typeahead.js:20:13:20:45 | target | typeahead.js:21:12:21:17 | target |
12871300
| typeahead.js:20:22:20:38 | document.location | typeahead.js:20:22:20:45 | documen ... .search |
12881301
| typeahead.js:20:22:20:38 | document.location | typeahead.js:20:22:20:45 | documen ... .search |
@@ -1491,6 +1504,7 @@ edges
14911504
| tst.js:417:18:417:24 | payload | tst.js:416:17:416:31 | window.location | tst.js:417:18:417:24 | payload | Cross-site scripting vulnerability due to $@. | tst.js:416:17:416:31 | window.location | user-provided value |
14921505
| tst.js:421:20:421:27 | match[1] | tst.js:419:15:419:29 | window.location | tst.js:421:20:421:27 | match[1] | Cross-site scripting vulnerability due to $@. | tst.js:419:15:419:29 | window.location | user-provided value |
14931506
| tst.js:424:18:424:51 | window. ... '#')[1] | tst.js:424:18:424:32 | window.location | tst.js:424:18:424:51 | window. ... '#')[1] | Cross-site scripting vulnerability due to $@. | tst.js:424:18:424:32 | window.location | user-provided value |
1507+
| tst.js:430:18:430:89 | target. ... data>') | tst.js:428:16:428:32 | document.location | tst.js:430:18:430:89 | target. ... data>') | Cross-site scripting vulnerability due to $@. | tst.js:428:16:428:32 | document.location | user-provided value |
14941508
| 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 |
14951509
| 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 |
14961510
| various-concat-obfuscations.js:4:4:4:31 | "<div>" ... </div>" | various-concat-obfuscations.js:2:16:2:39 | documen ... .search | various-concat-obfuscations.js:4:4:4:31 | "<div>" ... </div>" | Cross-site scripting vulnerability due to $@. | various-concat-obfuscations.js:2:16:2:39 | documen ... .search | user-provided value |

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,13 @@ nodes
660660
| tst.js:424:18:424:48 | window. ... it('#') |
661661
| tst.js:424:18:424:51 | window. ... '#')[1] |
662662
| tst.js:424:18:424:51 | window. ... '#')[1] |
663+
| tst.js:428:7:428:39 | target |
664+
| tst.js:428:16:428:32 | document.location |
665+
| tst.js:428:16:428:32 | document.location |
666+
| tst.js:428:16:428:39 | documen ... .search |
667+
| tst.js:430:18:430:23 | target |
668+
| tst.js:430:18:430:89 | target. ... data>') |
669+
| tst.js:430:18:430:89 | target. ... data>') |
663670
| typeahead.js:9:28:9:30 | loc |
664671
| typeahead.js:9:28:9:30 | loc |
665672
| typeahead.js:10:16:10:18 | loc |
@@ -1307,6 +1314,12 @@ edges
13071314
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') |
13081315
| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] |
13091316
| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] |
1317+
| tst.js:428:7:428:39 | target | tst.js:430:18:430:23 | target |
1318+
| tst.js:428:16:428:32 | document.location | tst.js:428:16:428:39 | documen ... .search |
1319+
| tst.js:428:16:428:32 | document.location | tst.js:428:16:428:39 | documen ... .search |
1320+
| tst.js:428:16:428:39 | documen ... .search | tst.js:428:7:428:39 | target |
1321+
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') |
1322+
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') |
13101323
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |
13111324
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |
13121325
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,3 +423,11 @@ function hash2() {
423423

424424
document.write(window.location.hash.split('#')[1]); // NOT OK
425425
}
426+
427+
function nonGlobalSanitizer() {
428+
var target = document.location.search
429+
430+
$("#foo").html(target.replace(/<metadata>[\s\S]*<\/metadata>/, '<metadata></metadata>')); // NOT OK
431+
432+
$("#foo").html(target.replace(/<|>/g, '')); // OK
433+
}

0 commit comments

Comments
 (0)