From 40cfbab335370d674fbf9b704381e0d88c473ce3 Mon Sep 17 00:00:00 2001
From: Esben Sparre Andreasen <esbena@github.com>
Date: Tue, 12 Jan 2021 08:25:16 +0100
Subject: [PATCH] JS: address review feedback

---
 .../IncompleteMultiCharacterSanitization.ql   |  9 ++-
 ...ompleteMultiCharacterSanitization.expected | 66 +++++++++----------
 2 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql b/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql
index cef10d3cbe7..3ae30ffe15b 100644
--- a/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql
+++ b/javascript/ql/src/Security/CWE-116/IncompleteMultiCharacterSanitization.ql
@@ -89,13 +89,13 @@ predicate matchesDangerousPrefix(EmptyReplaceRegExpTerm t, string prefix, string
     kind = "path injection" and
     // upwards navigation
     prefix = ["/..", "../"] and
-    not t.getSuccessor*().getAMatchedString().regexpMatch("(?i).*[a-z0-9_-]+.*") // explicit path name mentions make this an unlikely sanitizer
+    not t.getSuccessor*().getAMatchedString().regexpMatch("(?is).*[a-z0-9_-].*") // explicit path name mentions make this an unlikely sanitizer
     or
     kind = "HTML element injection" and
     (
       // comments
       prefix = "<!--" and
-      not t.getSuccessor*().getAMatchedString().regexpMatch("(?i).*[a-z0-9_]+.*") // explicit comment content mentions make this an unlikely sanitizer
+      not t.getSuccessor*().getAMatchedString().regexpMatch("(?is).*[a-z0-9_].*") // explicit comment content mentions make this an unlikely sanitizer
       or
       // specific tags
       prefix = "<" + ["iframe", "script", "cript", "scrip", "style"] // the `cript|scrip` case has been observed in the wild several times
@@ -159,6 +159,5 @@ where
   not replace.getAMethodCall*().flowsTo(replace.getReceiver()) and
   // avoid anchored terms
   not exists(RegExpAnchor a | regexp = a.getRootTerm())
-select replace,
-  "This string may still contain a substring that starts matching at $@, which may cause a " + kind +
-    " vulnerability.", dangerous, prefix
+select replace, "This string may still contain $@, which may cause a " + kind + " vulnerability.",
+  dangerous, prefix
diff --git a/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteMultiCharacterSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteMultiCharacterSanitization.expected
index 048109da744..9403b43cd43 100644
--- a/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteMultiCharacterSanitization.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-116/IncompleteSanitization/IncompleteMultiCharacterSanitization.expected
@@ -1,33 +1,33 @@
-| tst-multi-character-sanitization.js:3:13:3:57 | content ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:3:30:3:30 | < | <cript |
-| tst-multi-character-sanitization.js:4:13:4:47 | content ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:4:30:4:40 |  on\\w+=".*" | on |
-| tst-multi-character-sanitization.js:5:13:5:49 | content ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:5:30:5:42 |  on\\w+=\\'.*\\' | on |
-| tst-multi-character-sanitization.js:9:13:9:47 | content ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:9:30:9:30 | < | <cript |
-| tst-multi-character-sanitization.js:10:13:10:49 | content ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:10:30:10:42 | .on\\w+=.*".*" | on |
-| tst-multi-character-sanitization.js:11:13:11:51 | content ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:11:30:11:44 | .on\\w+=.*\\'.*\\' | on |
-| tst-multi-character-sanitization.js:19:3:19:35 | respons ... pt, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:18:18:18:24 | <script | <script |
-| tst-multi-character-sanitization.js:25:10:25:40 | text.re ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:25:24:25:27 | <!-- | <!-- |
-| tst-multi-character-sanitization.js:49:13:49:43 | req.url ... EL, "") | This string may still contain a substring that starts matching at $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:48:22:48:23 | \\/ | /.. |
-| tst-multi-character-sanitization.js:64:7:64:73 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:64:18:64:24 | <script | <script |
-| tst-multi-character-sanitization.js:66:7:66:56 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:66:18:66:49 | (\\/\|\\s)on\\w+=(\\'\|")?[^"]*(\\'\|")? | on |
-| tst-multi-character-sanitization.js:75:7:75:37 | x.repla ... gm, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:75:18:75:21 | <!-- | <!-- |
-| tst-multi-character-sanitization.js:76:7:76:35 | x.repla ... +/, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:76:18:76:29 | \\sng-[a-z-]+ | ng- |
-| tst-multi-character-sanitization.js:77:7:77:36 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:77:18:77:29 | \\sng-[a-z-]+ | ng- |
-| tst-multi-character-sanitization.js:81:7:81:58 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:81:36:81:39 | only | on |
-| tst-multi-character-sanitization.js:81:7:81:58 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:81:18:81:24 | <script | <script |
-| tst-multi-character-sanitization.js:83:7:83:63 | x.repla ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:83:18:83:21 | <!-- | <!-- |
-| tst-multi-character-sanitization.js:85:7:85:48 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:85:18:85:21 | \\x2E | ../ |
-| tst-multi-character-sanitization.js:87:7:87:47 | x.repla ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:87:18:87:24 | <script | <script |
-| tst-multi-character-sanitization.js:92:7:96:4 | x.repla ... ";\\n  }) | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:92:18:92:24 | <script | <script |
-| tst-multi-character-sanitization.js:101:7:101:30 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:101:18:101:19 | \\. | ../ |
-| tst-multi-character-sanitization.js:102:7:102:30 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:102:18:102:19 | \\/ | /.. |
-| tst-multi-character-sanitization.js:104:7:104:58 | x.repla ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:104:18:104:24 | <script | <script |
-| tst-multi-character-sanitization.js:106:7:106:64 | x.repla ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:106:18:106:18 | < | <script |
-| tst-multi-character-sanitization.js:107:7:107:62 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:107:18:107:19 | \\< | <script |
-| tst-multi-character-sanitization.js:108:7:108:75 | x.repla ... gm, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:108:18:108:18 | < | <script |
-| tst-multi-character-sanitization.js:109:7:109:58 | x.repla ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:109:18:109:24 | <script | <script |
-| tst-multi-character-sanitization.js:110:7:110:50 | x.repla ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:110:18:110:24 | <script | <script |
-| tst-multi-character-sanitization.js:111:7:111:32 | x.repla ... /g, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:111:18:111:19 |  ? | <!-- |
-| tst-multi-character-sanitization.js:126:7:129:34 | x\\n    . ... //, "") | This string may still contain a substring that starts matching at $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:129:15:129:20 | [^\\/]* | /.. |
-| tst-multi-character-sanitization.js:135:2:135:44 | content ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:135:19:135:25 | <script | <script |
-| tst-multi-character-sanitization.js:136:2:136:46 | content ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:136:19:136:19 | < | <script |
-| tst-multi-character-sanitization.js:138:2:138:48 | content ... gi, "") | This string may still contain a substring that starts matching at $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:138:19:138:20 | .* | <script |
+| tst-multi-character-sanitization.js:3:13:3:57 | content ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:3:30:3:30 | < | <cript |
+| tst-multi-character-sanitization.js:4:13:4:47 | content ... /g, "") | This string may still contain $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:4:30:4:40 |  on\\w+=".*" | on |
+| tst-multi-character-sanitization.js:5:13:5:49 | content ... /g, "") | This string may still contain $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:5:30:5:42 |  on\\w+=\\'.*\\' | on |
+| tst-multi-character-sanitization.js:9:13:9:47 | content ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:9:30:9:30 | < | <cript |
+| tst-multi-character-sanitization.js:10:13:10:49 | content ... /g, "") | This string may still contain $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:10:30:10:42 | .on\\w+=.*".*" | on |
+| tst-multi-character-sanitization.js:11:13:11:51 | content ... /g, "") | This string may still contain $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:11:30:11:44 | .on\\w+=.*\\'.*\\' | on |
+| tst-multi-character-sanitization.js:19:3:19:35 | respons ... pt, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:18:18:18:24 | <script | <script |
+| tst-multi-character-sanitization.js:25:10:25:40 | text.re ... /g, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:25:24:25:27 | <!-- | <!-- |
+| tst-multi-character-sanitization.js:49:13:49:43 | req.url ... EL, "") | This string may still contain $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:48:22:48:23 | \\/ | /.. |
+| tst-multi-character-sanitization.js:64:7:64:73 | x.repla ... /g, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:64:18:64:24 | <script | <script |
+| tst-multi-character-sanitization.js:66:7:66:56 | x.repla ... /g, "") | This string may still contain $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:66:18:66:49 | (\\/\|\\s)on\\w+=(\\'\|")?[^"]*(\\'\|")? | on |
+| tst-multi-character-sanitization.js:75:7:75:37 | x.repla ... gm, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:75:18:75:21 | <!-- | <!-- |
+| tst-multi-character-sanitization.js:76:7:76:35 | x.repla ... +/, "") | This string may still contain $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:76:18:76:29 | \\sng-[a-z-]+ | ng- |
+| tst-multi-character-sanitization.js:77:7:77:36 | x.repla ... /g, "") | This string may still contain $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:77:18:77:29 | \\sng-[a-z-]+ | ng- |
+| tst-multi-character-sanitization.js:81:7:81:58 | x.repla ... /g, "") | This string may still contain $@, which may cause a HTML attribute injection vulnerability. | tst-multi-character-sanitization.js:81:36:81:39 | only | on |
+| tst-multi-character-sanitization.js:81:7:81:58 | x.repla ... /g, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:81:18:81:24 | <script | <script |
+| tst-multi-character-sanitization.js:83:7:83:63 | x.repla ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:83:18:83:21 | <!-- | <!-- |
+| tst-multi-character-sanitization.js:85:7:85:48 | x.repla ... /g, "") | This string may still contain $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:85:18:85:21 | \\x2E | ../ |
+| tst-multi-character-sanitization.js:87:7:87:47 | x.repla ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:87:18:87:24 | <script | <script |
+| tst-multi-character-sanitization.js:92:7:96:4 | x.repla ... ";\\n  }) | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:92:18:92:24 | <script | <script |
+| tst-multi-character-sanitization.js:101:7:101:30 | x.repla ... /g, "") | This string may still contain $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:101:18:101:19 | \\. | ../ |
+| tst-multi-character-sanitization.js:102:7:102:30 | x.repla ... /g, "") | This string may still contain $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:102:18:102:19 | \\/ | /.. |
+| tst-multi-character-sanitization.js:104:7:104:58 | x.repla ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:104:18:104:24 | <script | <script |
+| tst-multi-character-sanitization.js:106:7:106:64 | x.repla ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:106:18:106:18 | < | <script |
+| tst-multi-character-sanitization.js:107:7:107:62 | x.repla ... /g, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:107:18:107:19 | \\< | <script |
+| tst-multi-character-sanitization.js:108:7:108:75 | x.repla ... gm, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:108:18:108:18 | < | <script |
+| tst-multi-character-sanitization.js:109:7:109:58 | x.repla ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:109:18:109:24 | <script | <script |
+| tst-multi-character-sanitization.js:110:7:110:50 | x.repla ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:110:18:110:24 | <script | <script |
+| tst-multi-character-sanitization.js:111:7:111:32 | x.repla ... /g, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:111:18:111:19 |  ? | <!-- |
+| tst-multi-character-sanitization.js:126:7:129:34 | x\\n    . ... //, "") | This string may still contain $@, which may cause a path injection vulnerability. | tst-multi-character-sanitization.js:129:15:129:20 | [^\\/]* | /.. |
+| tst-multi-character-sanitization.js:135:2:135:44 | content ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:135:19:135:25 | <script | <script |
+| tst-multi-character-sanitization.js:136:2:136:46 | content ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:136:19:136:19 | < | <script |
+| tst-multi-character-sanitization.js:138:2:138:48 | content ... gi, "") | This string may still contain $@, which may cause a HTML element injection vulnerability. | tst-multi-character-sanitization.js:138:19:138:20 | .* | <script |