Merge pull request github#11072 from erik-krogh/slicing

JS: poly-redos: don't sanitize calls through substring calls that just remove the start

Author: erik-krogh

javascript/ql/lib/semmle/javascript/security/regexp/PolynomialReDoSCustomizations.qll

@@ -90,7 +90,8 @@ module PolynomialReDoS {
         isCharClassLike(root)
       )
       or
-      this.(DataFlow::MethodCallNode).getMethodName() = StringOps::substringMethodName()
+      this.(DataFlow::MethodCallNode).getMethodName() = StringOps::substringMethodName() and
+      not this.(DataFlow::MethodCallNode).getNumArgument() = 1 // with one argument it just slices off the beginning
     }
   }
 

javascript/ql/test/query-tests/Security/CWE-400/ReDoS/PolynomialBackTracking.expected

@@ -33,6 +33,8 @@
 | lib/lib.js:8:3:8:4 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
 | lib/lib.js:28:3:28:4 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
 | lib/lib.js:36:3:36:4 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
+| lib/lib.js:42:29:42:30 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
+| lib/lib.js:45:29:45:30 | f* | Strings with many repetitions of 'f' can start matching anywhere after the start of the preceeding f*g |
 | lib/moduleLib/moduleLib.js:2:3:2:4 | a* | Strings with many repetitions of 'a' can start matching anywhere after the start of the preceeding a*b |
 | lib/otherLib/js/src/index.js:2:3:2:4 | a* | Strings with many repetitions of 'a' can start matching anywhere after the start of the preceeding a*b |
 | lib/snapdragon.js:7:28:7:29 | a* | Strings starting with 'a' and with many repetitions of 'a' can start matching anywhere after the start of the preceeding aa*$ |

javascript/ql/test/query-tests/Security/CWE-400/ReDoS/PolynomialReDoS.expected

@@ -28,6 +28,15 @@ nodes
 | lib/lib.js:35:28:35:31 | name |
 | lib/lib.js:36:13:36:16 | name |
 | lib/lib.js:36:13:36:16 | name |
+| lib/lib.js:41:32:41:35 | name |
+| lib/lib.js:41:32:41:35 | name |
+| lib/lib.js:42:17:42:20 | name |
+| lib/lib.js:42:17:42:20 | name |
+| lib/lib.js:44:5:44:25 | name |
+| lib/lib.js:44:12:44:15 | name |
+| lib/lib.js:44:12:44:25 | name.substr(1) |
+| lib/lib.js:45:17:45:20 | name |
+| lib/lib.js:45:17:45:20 | name |
 | lib/moduleLib/moduleLib.js:1:28:1:31 | name |
 | lib/moduleLib/moduleLib.js:1:28:1:31 | name |
 | lib/moduleLib/moduleLib.js:2:13:2:16 | name |
@@ -249,6 +258,16 @@ edges
 | lib/lib.js:35:1:37:1 | 'arguments' object of function usedWithArguments | lib/lib.js:35:28:35:31 | name |
 | lib/lib.js:35:28:35:31 | name | lib/lib.js:36:13:36:16 | name |
 | lib/lib.js:35:28:35:31 | name | lib/lib.js:36:13:36:16 | name |
+| lib/lib.js:41:32:41:35 | name | lib/lib.js:42:17:42:20 | name |
+| lib/lib.js:41:32:41:35 | name | lib/lib.js:42:17:42:20 | name |
+| lib/lib.js:41:32:41:35 | name | lib/lib.js:42:17:42:20 | name |
+| lib/lib.js:41:32:41:35 | name | lib/lib.js:42:17:42:20 | name |
+| lib/lib.js:41:32:41:35 | name | lib/lib.js:44:12:44:15 | name |
+| lib/lib.js:41:32:41:35 | name | lib/lib.js:44:12:44:15 | name |
+| lib/lib.js:44:5:44:25 | name | lib/lib.js:45:17:45:20 | name |
+| lib/lib.js:44:5:44:25 | name | lib/lib.js:45:17:45:20 | name |
+| lib/lib.js:44:12:44:15 | name | lib/lib.js:44:12:44:25 | name.substr(1) |
+| lib/lib.js:44:12:44:25 | name.substr(1) | lib/lib.js:44:5:44:25 | name |
 | lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name |
 | lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name |
 | lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name |
@@ -440,6 +459,8 @@ edges
 | lib/lib.js:4:2:4:18 | regexp.test(name) | lib/lib.js:3:28:3:31 | name | lib/lib.js:4:14:4:17 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/lib.js:1:15:1:16 | a* | regular expression | lib/lib.js:3:28:3:31 | name | library input |
 | lib/lib.js:8:2:8:17 | /f*g/.test(name) | lib/lib.js:7:19:7:22 | name | lib/lib.js:8:13:8:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/lib.js:8:3:8:4 | f* | regular expression | lib/lib.js:7:19:7:22 | name | library input |
 | lib/lib.js:36:2:36:17 | /f*g/.test(name) | lib/lib.js:32:32:32:40 | arguments | lib/lib.js:36:13:36:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/lib.js:36:3:36:4 | f* | regular expression | lib/lib.js:32:32:32:40 | arguments | library input |
+| lib/lib.js:42:17:42:33 | name.match(/f*g/) | lib/lib.js:41:32:41:35 | name | lib/lib.js:42:17:42:20 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/lib.js:42:29:42:30 | f* | regular expression | lib/lib.js:41:32:41:35 | name | library input |
+| lib/lib.js:45:17:45:33 | name.match(/f*g/) | lib/lib.js:41:32:41:35 | name | lib/lib.js:45:17:45:20 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'f'. | lib/lib.js:45:29:45:30 | f* | regular expression | lib/lib.js:41:32:41:35 | name | library input |
 | lib/moduleLib/moduleLib.js:2:2:2:17 | /a*b/.test(name) | lib/moduleLib/moduleLib.js:1:28:1:31 | name | lib/moduleLib/moduleLib.js:2:13:2:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/moduleLib/moduleLib.js:2:3:2:4 | a* | regular expression | lib/moduleLib/moduleLib.js:1:28:1:31 | name | library input |
 | lib/otherLib/js/src/index.js:2:2:2:17 | /a*b/.test(name) | lib/otherLib/js/src/index.js:1:28:1:31 | name | lib/otherLib/js/src/index.js:2:13:2:16 | name | This $@ that depends on $@ may run slow on strings with many repetitions of 'a'. | lib/otherLib/js/src/index.js:2:3:2:4 | a* | regular expression | lib/otherLib/js/src/index.js:1:28:1:31 | name | library input |
 | lib/snapdragon.js:7:15:7:32 | this.match(/aa*$/) | lib/snapdragon.js:3:34:3:38 | input | lib/snapdragon.js:7:15:7:18 | this | This $@ that depends on $@ may run slow on strings starting with 'a' and with many repetitions of 'a'. | lib/snapdragon.js:7:28:7:29 | a* | regular expression | lib/snapdragon.js:3:34:3:38 | input | library input |

javascript/ql/test/query-tests/Security/CWE-400/ReDoS/lib/lib.js

@@ -36,4 +36,11 @@ function usedWithArguments(name) {
 	/f*g/.test(name); // NOT OK
 }
 
-module.exports.snapdragon = require("./snapdragon")
+module.exports.snapdragon = require("./snapdragon")
+
+module.exports.foo = function (name) {
+    var data1 = name.match(/f*g/); // NOT OK
+
+    name = name.substr(1);
+    var data2 = name.match(/f*g/); // NOT OK
+}