prune results that end with newline, where the input cannot contain newlines

Author: erik-krogh

javascript/ql/src/Performance/PolynomialReDoS.ql

@@ -14,9 +14,15 @@
 
 import javascript
 import semmle.javascript.security.performance.PolynomialReDoS::PolynomialReDoS
+import semmle.javascript.security.performance.SuperlinearBackTracking
 import DataFlow::PathGraph
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
-where cfg.hasFlowPath(source, sink)
+where
+  cfg.hasFlowPath(source, sink) and
+  not (
+    source.getNode().(Source).getKind() = "url" and
+    sink.getNode().(Sink).getRegExp().(PolynomialBackTrackingTerm).isAtEndLine()
+  )
 select sink.getNode(), source, sink, "This expensive $@ use depends on $@.",
   sink.getNode().(Sink).getRegExp(), "regular expression", source.getNode(), "a user-provided value"

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

@@ -11,7 +11,13 @@ module PolynomialReDoS {
   /**
    * A data flow source node for polynomial regular expression denial-of-service vulnerabilities.
    */
-  abstract class Source extends DataFlow::Node { }
+  abstract class Source extends DataFlow::Node {
+    /**
+     * Gets the kind of source that is being accesed. See `HTTP::RequestInputAccess::getKind()`. 
+     * Can be one of "parameter", "header", "body", "url", "cookie".
+     */
+    abstract string getKind();
+  }
 
   /**
    * A data flow sink node for polynomial regular expression denial-of-service vulnerabilities.
@@ -31,6 +37,10 @@ module PolynomialReDoS {
    */
   class RequestInputAccessAsSource extends Source {
     RequestInputAccessAsSource() { this instanceof HTTP::RequestInputAccess }
+
+    override string getKind() {
+      result = this.(HTTP::RequestInputAccess).getKind()
+    }
   }
 
   /**

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

@@ -146,6 +146,15 @@ class PolynomialBackTrackingTerm extends InfiniteRepetitionQuantifier {
     not this.getParent*() instanceof RegExpSubPattern // too many corner cases
   }
 
+  /**
+   * Holds if all non-empty successors to the polynimial backtracking term matches the end of the line.
+   */
+  predicate isAtEndLine() {
+    forall(RegExpTerm succ | this.getSuccessor+() = succ and not matchesEpsilon(succ) |
+      succ instanceof RegExpDollar
+    )
+  }
+
   /**
    * Gets the reason for the number of match attempts.
    */

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

@@ -66,6 +66,12 @@ nodes
 | polynomial-redos.js:66:19:66:25 | tainted |
 | polynomial-redos.js:67:18:67:24 | tainted |
 | polynomial-redos.js:67:18:67:24 | tainted |
+| polynomial-redos.js:68:18:68:24 | req.url |
+| 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:69:18:69:25 | req.body |
 edges
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:7:2:7:8 | tainted |
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:7:2:7:8 | tainted |
@@ -133,6 +139,8 @@ edges
 | polynomial-redos.js:5:6:5:32 | tainted | polynomial-redos.js:67:18:67: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 |
 #select
 | 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 |
 | polynomial-redos.js:8:2:8:8 | tainted | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:8:2:8:8 | tainted | This expensive $@ use depends on $@. | polynomial-redos.js:8:17:8:18 |  * | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
@@ -170,3 +178,4 @@ edges
 | polynomial-redos.js:65:24:65:30 | tainted | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:65:24:65:30 | tainted | This expensive $@ use depends on $@. | polynomial-redos.js:65:14:65:15 | .* | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
 | polynomial-redos.js:66:19:66:25 | tainted | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:66:19:66:25 | tainted | This expensive $@ use depends on $@. | polynomial-redos.js:66:9:66:10 | .* | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
 | polynomial-redos.js:67:18:67:24 | tainted | polynomial-redos.js:5:16:5:32 | req.query.tainted | polynomial-redos.js:67:18:67:24 | tainted | This expensive $@ use depends on $@. | polynomial-redos.js:67:8:67:9 | .* | regular expression | polynomial-redos.js:5:16:5:32 | req.query.tainted | a user-provided value |
+| polynomial-redos.js:69:18:69:25 | req.body | polynomial-redos.js:69:18:69:25 | req.body | polynomial-redos.js:69:18:69:25 | req.body | This expensive $@ use depends on $@. | polynomial-redos.js:69:8:69:9 | .* | regular expression | polynomial-redos.js:69:18:69:25 | req.body | a user-provided value |

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

@@ -65,4 +65,6 @@ app.use(function(req, res) {
 	(/^foo(K|Y)+.*X/.test(tainted)); // NOT OK
 	(/(K|Y).*X/.test(tainted)); // NOT OK
 	(/[^Y].*X/.test(tainted)); // NOT OK
+	(/[^Y].*$/.test(req.url)); // OK - the input cannot contain newlines.
+	(/[^Y].*$/.test(req.body)); // NOT OK
 });