dont sanitize global replacements where the regexp is a char class

Author: erik-krogh

javascript/ql/src/semmle/javascript/security/performance/PolynomialReDoSCustomizations.qll

@@ -84,7 +84,13 @@ module PolynomialReDoS {
    */
   class StringLengthLimiter extends Sanitizer {
     StringLengthLimiter() {
-      this.(StringReplaceCall).isGlobal()
+      this.(StringReplaceCall).isGlobal() and
+      // not char classes - they don't remove any repeated pattern.
+      not exists(RegExpTerm root | root = this.(StringReplaceCall).getRegExp().getRoot() |
+        root instanceof RegExpCharacterClass
+        or
+        root instanceof RegExpCharacterClassEscape
+      )
       or
       exists(string name | name = "slice" or name = "substring" or name = "substr" |
         this.(DataFlow::MethodCallNode).getMethodName() = name

javascript/ql/test/query-tests/Performance/ReDoS/PolynomialBackTracking.expected

@@ -104,6 +104,7 @@
 | polynomial-redos.js:114:22:114:24 | \\w* | Strings starting with '5' and with many repetitions of '5' can start matching anywhere after the start of the preceeding \\d* |
 | polynomial-redos.js:116:21:116:28 | [\\d\\D]*? | Strings starting with '/*' and with many repetitions of 'a/*' can start matching anywhere after the start of the preceeding \\/\\*[\\d\\D]*?\\*\\/ |
 | polynomial-redos.js:118:17:118:23 | (#\\d+)+ | Strings with many repetitions of '9' can start matching anywhere after the start of the preceeding \\d+ |
+| polynomial-redos.js:124:33:124:35 | \\s+ | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding \\s+$ |
 | regexplib/address.js:27:3:27:5 | \\s* | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding (\\s*\\(?0\\d{4}\\)?(\\s*\|-)\\d{3}(\\s*\|-)\\d{3}\\s*) |
 | regexplib/address.js:27:48:27:50 | \\s* | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding (\\s*\\(?0\\d{3}\\)?(\\s*\|-)\\d{3}(\\s*\|-)\\d{4}\\s*) |
 | regexplib/address.js:27:93:27:95 | \\s* | Strings with many repetitions of ' ' can start matching anywhere after the start of the preceeding (\\s*(7\|8)(\\d{7}\|\\d{3}(\\-\|\\s{1})\\d{4})\\s*) |

javascript/ql/test/query-tests/Performance/ReDoS/PolynomialReDoS.expected

@@ -138,6 +138,13 @@ nodes
 | polynomial-redos.js:116:2:116:8 | tainted |
 | polynomial-redos.js:118:2:118:8 | tainted |
 | polynomial-redos.js:118:2:118:8 | tainted |
+| polynomial-redos.js:121:7:121:55 | replaced |
+| polynomial-redos.js:121:18:121:24 | tainted |
+| polynomial-redos.js:121:18:121:55 | tainted ... /g, '') |
+| polynomial-redos.js:123:3:123:20 | result |
+| polynomial-redos.js:123:13:123:20 | replaced |
+| polynomial-redos.js:124:12:124:17 | result |
+| polynomial-redos.js:124:12:124:17 | result |
 edges
 | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name |
 | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name |
@@ -269,10 +276,17 @@ edges
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:116:2:116:8 | tainted |
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:118:2:118:8 | tainted |
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:118:2:118:8 | tainted |
+| polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:121:18:121:24 | tainted |
 | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:5:6:5:32 | tainted |
 | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:5:6:5:32 | tainted |
 | polynomial-redos.js:68:18:68:24 | req.url | polynomial-redos.js:68:18:68:24 | req.url |
 | polynomial-redos.js:69:18:69:25 | req.body | polynomial-redos.js:69:18:69:25 | req.body |
+| polynomial-redos.js:121:7:121:55 | replaced | polynomial-redos.js:123:13:123:20 | replaced |
+| polynomial-redos.js:121:18:121:24 | tainted | polynomial-redos.js:121:18:121:55 | tainted ... /g, '') |
+| polynomial-redos.js:121:18:121:55 | tainted ... /g, '') | polynomial-redos.js:121:7:121:55 | replaced |
+| polynomial-redos.js:123:3:123:20 | result | polynomial-redos.js:124:12:124:17 | result |
+| polynomial-redos.js:123:3:123:20 | result | polynomial-redos.js:124:12:124:17 | result |
+| polynomial-redos.js:123:13:123:20 | replaced | polynomial-redos.js:123:3:123:20 | result |
 #select
 | lib/lib.js:4:14:4:17 | name | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | This expensive $@ use depends on $@. | lib/lib.js:1:15:1:16 | a* | regular expression | lib/lib.js:3:28:3:31 | name | library input |
 | polynomial-redos.js:7:2:7:8 | tainted | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:7:2:7:8 | tainted | This expensive $@ use depends on $@. | polynomial-redos.js:7:24:7:26 | \\s+ | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
@@ -349,3 +363,4 @@ edges
 | polynomial-redos.js:114:2:114:8 | tainted | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:114:2:114:8 | tainted | This expensive $@ use depends on $@. | polynomial-redos.js:114:22:114:24 | \\w* | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
 | polynomial-redos.js:116:2:116:8 | tainted | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:116:2:116:8 | tainted | This expensive $@ use depends on $@. | polynomial-redos.js:116:21:116:28 | [\\d\\D]*? | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
 | polynomial-redos.js:118:2:118:8 | tainted | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:118:2:118:8 | tainted | This expensive $@ use depends on $@. | polynomial-redos.js:118:17:118:23 | (#\\d+)+ | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
+| polynomial-redos.js:124:12:124:17 | result | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:124:12:124:17 | result | This expensive $@ use depends on $@. | polynomial-redos.js:124:33:124:35 | \\s+ | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |

javascript/ql/test/query-tests/Performance/ReDoS/polynomial-redos.js

@@ -116,4 +116,11 @@ app.use(function(req, res) {
 	tainted.match(/\/\*[\d\D]*?\*\//g); // NOT OK
 
 	tainted.match(/(#\d+)+/); // OK - but still flagged due to insufficient suffix-checking.
+
+	(function foo() {
+		var replaced = tainted.replace(/[^\w\s\-\.\_~]/g, '');
+		var result = ""
+		result += replaced;
+		result = result.replace(/^\s+|\s+$/g, ''); // NOT OK
+	})();
 });