Refine polynomial redos sources to exclude length limited methods

This commit is contained in:
Joe Farebrother
2022-03-16 15:59:41 +00:00
parent 04edc10f1e
commit 1605d36ddf
4 changed files with 38 additions and 6 deletions

View File

@@ -1,4 +1,4 @@
/** Definitions and configurations for the Polynomial ReDos query */
/** Definitions and configurations for the Polynomial ReDoS query */
import semmle.code.java.security.performance.SuperlinearBackTracking
import semmle.code.java.dataflow.DataFlow
@@ -16,6 +16,22 @@ class PolynomialRedosSink extends DataFlow::Node {
RegExpTerm getRegExp() { result.getParent() = reg }
}
/**
* A method whose result typically has a limited length,
* such as HTTP headers, and values derrived from them.
*/
private class LengthRestrictedMethod extends Method {
LengthRestrictedMethod() {
this.getName().toLowerCase().matches(["%header%", "%requesturi%", "%requesturl%", "%cookie%"])
or
this.getDeclaringType().getName().toLowerCase().matches("%cookie%") and
this.getName().matches("get%")
or
this.getDeclaringType().getName().toLowerCase().matches("%request%") and
this.getName().toLowerCase().matches(["%get%path%", "get%user%", "%querystring%"])
}
}
/** A configuration for Polynomial ReDoS queries. */
class PolynomialRedosConfig extends TaintTracking::Configuration {
PolynomialRedosConfig() { this = "PolynomialRedosConfig" }
@@ -23,10 +39,17 @@ class PolynomialRedosConfig extends TaintTracking::Configuration {
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) { sink instanceof PolynomialRedosSink }
override predicate isSanitizer(DataFlow::Node node) {
super.isSanitizer(node) or
node.getType() instanceof PrimitiveType or
node.getType() instanceof BoxedType or
node.asExpr().(MethodAccess).getMethod() instanceof LengthRestrictedMethod
}
}
/** Holds if there is flow from `source` to `sink` that is matched against the regexp term `regexp` that is vulnerable to Polynomial ReDoS. */
predicate hasPolynomialReDosResult(
predicate hasPolynomialReDoSResult(
DataFlow::PathNode source, DataFlow::PathNode sink, PolynomialBackTrackingTerm regexp
) {
any(PolynomialRedosConfig config).hasFlowPath(source, sink) and

View File

@@ -12,11 +12,11 @@
*/
import java
import semmle.code.java.security.performance.PolynomialReDosQuery
import semmle.code.java.security.performance.PolynomialReDoSQuery
import DataFlow::PathGraph
from DataFlow::PathNode source, DataFlow::PathNode sink, PolynomialBackTrackingTerm regexp
where hasPolynomialReDosResult(source, sink, regexp)
where hasPolynomialReDoSResult(source, sink, regexp)
select sink, source, sink,
"This $@ that depends on $@ may run slow on strings " + regexp.getPrefixMessage() +
"with many repetitions of '" + regexp.getPumpString() + "'.", regexp, "regular expression",

View File

@@ -72,4 +72,13 @@ class PolyRedosTest {
p3.asMatchPredicate().test(tainted);
p4.asPredicate().test(tainted); // $ hasPolyRedos
}
void test6(HttpServletRequest request) {
Pattern p = Pattern.compile("^a*a*$");
p.matcher(request.getParameter("inp")).matches(); // $ hasPolyRedos
p.matcher(request.getHeader("If-None-Match")).matches();
p.matcher(request.getRequestURI()).matches();
p.matcher(request.getCookies()[0].getName()).matches();
}
}

View File

@@ -1,6 +1,6 @@
import java
import TestUtilities.InlineExpectationsTest
import semmle.code.java.security.performance.PolynomialReDosQuery
import semmle.code.java.security.performance.PolynomialReDoSQuery
class HasPolyRedos extends InlineExpectationsTest {
HasPolyRedos() { this = "HasPolyRedos" }
@@ -10,7 +10,7 @@ class HasPolyRedos extends InlineExpectationsTest {
override predicate hasActualResult(Location location, string element, string tag, string value) {
tag = "hasPolyRedos" and
exists(DataFlow::PathNode source, DataFlow::PathNode sink, PolynomialBackTrackingTerm regexp |
hasPolynomialReDosResult(source, sink, regexp) and
hasPolynomialReDoSResult(source, sink, regexp) and
location = sink.getNode().getLocation() and
element = sink.getNode().toString() and
value = ""