Skip to content

Commit c71778f

Browse files
NapalysNapalys
committed
JS: xss does not flag anymore replace with RegExp unknown flags
1 parent dbae553 commit c71778f

File tree

5 files changed

+2
-16
lines changed

5 files changed

+2
-16
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ module Shared {
3636
*/
3737
class MetacharEscapeSanitizer extends Sanitizer, StringReplaceCall {
3838
MetacharEscapeSanitizer() {
39-
this.isGlobal() and
39+
this.maybeGlobal() and
4040
(
4141
RegExp::alwaysMatchesMetaCharacter(this.getRegExp().getRoot(), ["<", "'", "\""])
4242
or

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +0,0 @@
1-
| tst.js:510 | did not expect an alert, but found an alert for HtmlInjection | OK -- currently flagged, but might introduce a lot of false positives. | |

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,9 +1154,6 @@ nodes
11541154
| tst.js:509:18:509:23 | target |
11551155
| tst.js:509:18:509:54 | target. ... "), '') |
11561156
| tst.js:509:18:509:54 | target. ... "), '') |
1157-
| tst.js:510:18:510:23 | target |
1158-
| tst.js:510:18:510:70 | target. ... )), '') |
1159-
| tst.js:510:18:510:70 | target. ... )), '') |
11601157
| typeahead.js:20:13:20:45 | target |
11611158
| typeahead.js:20:22:20:45 | documen ... .search |
11621159
| typeahead.js:20:22:20:45 | documen ... .search |
@@ -2341,13 +2338,10 @@ edges
23412338
| tst.js:501:43:501:62 | window.location.hash | tst.js:501:33:501:63 | decodeU ... n.hash) |
23422339
| tst.js:501:43:501:62 | window.location.hash | tst.js:501:33:501:63 | decodeU ... n.hash) |
23432340
| tst.js:508:7:508:39 | target | tst.js:509:18:509:23 | target |
2344-
| tst.js:508:7:508:39 | target | tst.js:510:18:510:23 | target |
23452341
| tst.js:508:16:508:39 | documen ... .search | tst.js:508:7:508:39 | target |
23462342
| tst.js:508:16:508:39 | documen ... .search | tst.js:508:7:508:39 | target |
23472343
| tst.js:509:18:509:23 | target | tst.js:509:18:509:54 | target. ... "), '') |
23482344
| tst.js:509:18:509:23 | target | tst.js:509:18:509:54 | target. ... "), '') |
2349-
| tst.js:510:18:510:23 | target | tst.js:510:18:510:70 | target. ... )), '') |
2350-
| tst.js:510:18:510:23 | target | tst.js:510:18:510:70 | target. ... )), '') |
23512345
| typeahead.js:20:13:20:45 | target | typeahead.js:21:12:21:17 | target |
23522346
| typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:20:13:20:45 | target |
23532347
| typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:20:13:20:45 | target |
@@ -2641,7 +2635,6 @@ edges
26412635
| tst.js:494:18:494:40 | locatio ... bstr(1) | tst.js:494:18:494:30 | location.hash | tst.js:494:18:494:40 | locatio ... bstr(1) | Cross-site scripting vulnerability due to $@. | tst.js:494:18:494:30 | location.hash | user-provided value |
26422636
| tst.js:501:33:501:63 | decodeU ... n.hash) | tst.js:501:43:501:62 | window.location.hash | tst.js:501:33:501:63 | decodeU ... n.hash) | Cross-site scripting vulnerability due to $@. | tst.js:501:43:501:62 | window.location.hash | user-provided value |
26432637
| tst.js:509:18:509:54 | target. ... "), '') | tst.js:508:16:508:39 | documen ... .search | tst.js:509:18:509:54 | target. ... "), '') | Cross-site scripting vulnerability due to $@. | tst.js:508:16:508:39 | documen ... .search | user-provided value |
2644-
| tst.js:510:18:510:70 | target. ... )), '') | tst.js:508:16:508:39 | documen ... .search | tst.js:510:18:510:70 | target. ... )), '') | Cross-site scripting vulnerability due to $@. | tst.js:508:16:508:39 | documen ... .search | user-provided value |
26452638
| typeahead.js:25:18:25:20 | val | typeahead.js:20:22:20:45 | documen ... .search | typeahead.js:25:18:25:20 | val | Cross-site scripting vulnerability due to $@. | typeahead.js:20:22:20:45 | documen ... .search | user-provided value |
26462639
| 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 |
26472640
| 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: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,9 +1166,6 @@ nodes
11661166
| tst.js:509:18:509:23 | target |
11671167
| tst.js:509:18:509:54 | target. ... "), '') |
11681168
| tst.js:509:18:509:54 | target. ... "), '') |
1169-
| tst.js:510:18:510:23 | target |
1170-
| tst.js:510:18:510:70 | target. ... )), '') |
1171-
| tst.js:510:18:510:70 | target. ... )), '') |
11721169
| typeahead.js:9:28:9:30 | loc |
11731170
| typeahead.js:9:28:9:30 | loc |
11741171
| typeahead.js:9:28:9:30 | loc |
@@ -2403,13 +2400,10 @@ edges
24032400
| tst.js:501:43:501:62 | window.location.hash | tst.js:501:33:501:63 | decodeU ... n.hash) |
24042401
| tst.js:501:43:501:62 | window.location.hash | tst.js:501:33:501:63 | decodeU ... n.hash) |
24052402
| tst.js:508:7:508:39 | target | tst.js:509:18:509:23 | target |
2406-
| tst.js:508:7:508:39 | target | tst.js:510:18:510:23 | target |
24072403
| tst.js:508:16:508:39 | documen ... .search | tst.js:508:7:508:39 | target |
24082404
| tst.js:508:16:508:39 | documen ... .search | tst.js:508:7:508:39 | target |
24092405
| tst.js:509:18:509:23 | target | tst.js:509:18:509:54 | target. ... "), '') |
24102406
| tst.js:509:18:509:23 | target | tst.js:509:18:509:54 | target. ... "), '') |
2411-
| tst.js:510:18:510:23 | target | tst.js:510:18:510:70 | target. ... )), '') |
2412-
| tst.js:510:18:510:23 | target | tst.js:510:18:510:70 | target. ... )), '') |
24132407
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |
24142408
| typeahead.js:9:28:9:30 | loc | typeahead.js:10:16:10:18 | loc |
24152409
| 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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,6 @@ function Foo() {
507507
function nonGlobalSanitizer() {
508508
var target = document.location.search
509509
$("#foo").html(target.replace(new RegExp("<|>"), '')); // NOT OK
510-
$("#foo").html(target.replace(new RegExp("<|>", unknownFlags()), '')); // OK -- currently flagged, but might introduce a lot of false positives.
510+
$("#foo").html(target.replace(new RegExp("<|>", unknownFlags()), '')); // OK -- most likely good. We don't know what the flags are.
511511
$("#foo").html(target.replace(new RegExp("<|>", "g"), '')); // OK
512512
}

0 commit comments

Comments
 (0)