Java: Promote regex injection query from experimental #11070

jcogs33 · 2022-11-01T19:56:34Z

This PR promotes #5704 from experimental.

github-actions · 2022-11-01T19:57:29Z

QHelp previews:

java/ql/src/Security/CWE/CWE-730/RegexInjection.qhelp

Regular expression injection

Constructing a regular expression with unsanitized user input is dangerous as a malicious user may be able to modify the meaning of the expression. In particular, such a user may be able to provide a regular expression fragment that takes exponential time in the worst case, and use that to perform a Denial of Service attack.

Recommendation

Before embedding user input into a regular expression, use a sanitization function such as Pattern.quote to escape meta-characters that have special meaning.

Example

The following example shows an HTTP request parameter that is used to construct a regular expression.

In the first case the user-provided regex is not escaped. If a malicious user provides a regex that has exponential worst case performance, then this could lead to a Denial of Service.

In the second case, the user input is escaped using Pattern.quote before being included in the regular expression. This ensures that the user cannot insert characters which have a special meaning in regular expressions.

import java.util.regex.Pattern;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;

public class RegexInjectionDemo extends HttpServlet {

  public boolean badExample(javax.servlet.http.HttpServletRequest request) {
    String regex = request.getParameter("regex");
    String input = request.getParameter("input");

    // BAD: Unsanitized user input is used to construct a regular expression
    return input.matches(regex);
  }

  public boolean goodExample(javax.servlet.http.HttpServletRequest request) {
    String regex = request.getParameter("regex");
    String input = request.getParameter("input");

    // GOOD: User input is sanitized before constructing the regex
    return input.matches(Pattern.quote(regex));
  }
}

References

OWASP: Regular expression Denial of Service - ReDoS.
Wikipedia: ReDoS.
Java API Specification: Pattern.quote.
Common Weakness Enumeration: CWE-730.
Common Weakness Enumeration: CWE-400.

java/ql/lib/semmle/code/java/frameworks/Regex.qll

github-actions · 2022-11-03T17:20:32Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons Lang <https://commons.apache.org/proper/commons-lang/>`_,``org.apache.commons.lang3``,,424,,,,,,,,
+    `Apache Commons Lang <https://commons.apache.org/proper/commons-lang/>`_,``org.apache.commons.lang3``,,424,6,,,,,,,
-    Totals,,217,8432,1524,129,6,10,107,33,1,86
+    Totals,,217,8432,1530,129,6,10,107,33,1,86

Changes to framework-coverage-java.csv:

- org.apache.commons.lang3,,,424,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,293,131
+ org.apache.commons.lang3,6,,424,,,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,293,131

github-actions · 2022-11-03T17:28:54Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons Lang <https://commons.apache.org/proper/commons-lang/>`_,``org.apache.commons.lang3``,,424,,,,,,,,
+    `Apache Commons Lang <https://commons.apache.org/proper/commons-lang/>`_,``org.apache.commons.lang3``,,424,6,,,,,,,
-    Totals,,217,8432,1524,129,6,10,107,33,1,86
+    Totals,,217,8432,1530,129,6,10,107,33,1,86

Changes to framework-coverage-java.csv:

- org.apache.commons.lang3,,,424,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,293,131
+ org.apache.commons.lang3,6,,424,,,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,293,131

github-actions · 2022-11-03T18:29:56Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons Lang <https://commons.apache.org/proper/commons-lang/>`_,``org.apache.commons.lang3``,,424,,,,,,,,
+    `Apache Commons Lang <https://commons.apache.org/proper/commons-lang/>`_,``org.apache.commons.lang3``,,424,6,,,,,,,
-    Totals,,217,8432,1524,129,6,10,107,33,1,86
+    Totals,,217,8432,1530,129,6,10,107,33,1,86

Changes to framework-coverage-java.csv:

- org.apache.commons.lang3,,,424,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,293,131
+ org.apache.commons.lang3,6,,424,,,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,293,131

github-actions · 2022-11-04T15:47:17Z

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

java

Generated file changes for java

Changes to framework-coverage-java.rst:

-    `Apache Commons Lang <https://commons.apache.org/proper/commons-lang/>`_,``org.apache.commons.lang3``,,424,,,,,,,,
+    `Apache Commons Lang <https://commons.apache.org/proper/commons-lang/>`_,``org.apache.commons.lang3``,,424,6,,,,,,,
-    Totals,,217,8432,1524,129,6,10,107,33,1,86
+    Totals,,217,8432,1530,129,6,10,107,33,1,86

Changes to framework-coverage-java.csv:

- package,sink,source,summary,sink:bean-validation,sink:create-file,sink:groovy,sink:header-splitting,sink:information-leak,sink:intent-start,sink:jdbc-url,sink:jexl,sink:jndi-injection,sink:ldap,sink:logging,sink:mvel,sink:ognl-injection,sink:open-url,sink:pending-intent-sent,sink:regex-use[-1],sink:regex-use[0],sink:regex-use[],sink:regex-use[f-1],sink:regex-use[f1],sink:regex-use[f],sink:set-hostname-verifier,sink:sql,sink:ssti,sink:url-open-stream,sink:url-redirect,sink:write-file,sink:xpath,sink:xslt,sink:xss,source:android-external-storage-dir,source:android-widget,source:contentprovider,source:remote,summary:taint,summary:value
+ package,sink,source,summary,sink:bean-validation,sink:create-file,sink:groovy,sink:header-splitting,sink:information-leak,sink:intent-start,sink:jdbc-url,sink:jexl,sink:jndi-injection,sink:ldap,sink:logging,sink:mvel,sink:ognl-injection,sink:open-url,sink:pending-intent-sent,sink:regex-use,sink:regex-use[-1],sink:regex-use[0],sink:regex-use[],sink:regex-use[f-1],sink:regex-use[f1],sink:regex-use[f],sink:set-hostname-verifier,sink:sql,sink:ssti,sink:url-open-stream,sink:url-redirect,sink:write-file,sink:xpath,sink:xslt,sink:xss,source:android-external-storage-dir,source:android-widget,source:contentprovider,source:remote,summary:taint,summary:value
- android.app,24,,103,,,,,,7,,,,,,,,,17,,,,,,,,,,,,,,,,,,,,18,85
+ android.app,24,,103,,,,,,7,,,,,,,,,17,,,,,,,,,,,,,,,,,,,,,18,85
- android.content,24,31,154,,,,,,16,,,,,,,,,,,,,,,,,8,,,,,,,,4,,27,,63,91
+ android.content,24,31,154,,,,,,16,,,,,,,,,,,,,,,,,,8,,,,,,,,4,,27,,63,91
- android.database,59,,39,,,,,,,,,,,,,,,,,,,,,,,59,,,,,,,,,,,,39,
+ android.database,59,,39,,,,,,,,,,,,,,,,,,,,,,,,59,,,,,,,,,,,,39,
- android.net,,,60,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,45,15
+ android.net,,,60,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,45,15
- android.os,,2,122,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,41,81
+ android.os,,2,122,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,41,81
- android.util,6,16,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,16,,
+ android.util,6,16,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,,16,,
- android.webkit,3,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,,,,2,,
+ android.webkit,3,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,,,,2,,
- android.widget,,1,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,1,
+ android.widget,,1,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,1,
- androidx.core.app,6,,95,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,12,83
+ androidx.core.app,6,,95,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,12,83
- androidx.slice,2,5,88,,,,,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,5,,27,61
+ androidx.slice,2,5,88,,,,,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,5,,27,61
- cn.hutool.core.codec,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
+ cn.hutool.core.codec,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
- com.esotericsoftware.kryo.io,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
+ com.esotericsoftware.kryo.io,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
- com.esotericsoftware.kryo5.io,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
+ com.esotericsoftware.kryo5.io,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
- com.fasterxml.jackson.core,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
+ com.fasterxml.jackson.core,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
- com.fasterxml.jackson.databind,,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,
+ com.fasterxml.jackson.databind,,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,
- com.google.common.base,4,,85,,,,,,,,,,,,,,,,,3,1,,,,,,,,,,,,,,,,,62,23
+ com.google.common.base,4,,85,,,,,,,,,,,,,,,,,,3,1,,,,,,,,,,,,,,,,,62,23
- com.google.common.cache,,,17,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,17
+ com.google.common.cache,,,17,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,17
- com.google.common.collect,,,553,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,551
+ com.google.common.collect,,,553,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,551
- com.google.common.flogger,29,,,,,,,,,,,,,29,,,,,,,,,,,,,,,,,,,,,,,,,
+ com.google.common.flogger,29,,,,,,,,,,,,,29,,,,,,,,,,,,,,,,,,,,,,,,,,
- com.google.common.io,6,,73,,,,,,,,,,,,,,,,,,,,,,,,,6,,,,,,,,,,72,1
+ com.google.common.io,6,,73,,,,,,,,,,,,,,,,,,,,,,,,,,6,,,,,,,,,,72,1
- com.hubspot.jinjava,2,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,,,,
+ com.hubspot.jinjava,2,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,,,,
- com.mitchellbosecke.pebble,2,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,,,,
+ com.mitchellbosecke.pebble,2,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,,,,
- com.opensymphony.xwork2.ognl,3,,,,,,,,,,,,,,,3,,,,,,,,,,,,,,,,,,,,,,,
+ com.opensymphony.xwork2.ognl,3,,,,,,,,,,,,,,,3,,,,,,,,,,,,,,,,,,,,,,,,
- com.rabbitmq.client,,21,7,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,21,7,
+ com.rabbitmq.client,,21,7,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,21,7,
- com.unboundid.ldap.sdk,17,,,,,,,,,,,,17,,,,,,,,,,,,,,,,,,,,,,,,,,
+ com.unboundid.ldap.sdk,17,,,,,,,,,,,,17,,,,,,,,,,,,,,,,,,,,,,,,,,,
- com.zaxxer.hikari,2,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ com.zaxxer.hikari,2,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- flexjson,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1
+ flexjson,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1
- freemarker.cache,1,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,,,,,,,,,,
+ freemarker.cache,1,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,,,,,,,,,,
- freemarker.template,7,,,,,,,,,,,,,,,,,,,,,,,,,,7,,,,,,,,,,,,
+ freemarker.template,7,,,,,,,,,,,,,,,,,,,,,,,,,,,7,,,,,,,,,,,,
- groovy.lang,26,,,,,26,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ groovy.lang,26,,,,,26,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- groovy.util,5,,,,,5,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ groovy.util,5,,,,,5,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- jakarta.faces.context,2,7,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,7,,
+ jakarta.faces.context,2,7,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,7,,
- jakarta.json,,,123,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,100,23
+ jakarta.json,,,123,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,100,23
- jakarta.ws.rs.client,1,,,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,
+ jakarta.ws.rs.client,1,,,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,
- jakarta.ws.rs.container,,9,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,9,,
+ jakarta.ws.rs.container,,9,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,9,,
- jakarta.ws.rs.core,2,,149,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,94,55
+ jakarta.ws.rs.core,2,,149,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,94,55
- java.beans,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
+ java.beans,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
- java.io,37,,40,,15,,,,,,,,,,,,,,,,,,,,,,,,,22,,,,,,,,40,
+ java.io,37,,40,,15,,,,,,,,,,,,,,,,,,,,,,,,,,22,,,,,,,,40,
- java.lang,13,,66,,,,,,,,,,,8,,,,,4,,,1,,,,,,,,,,,,,,,,54,12
+ java.lang,13,,66,,,,,,,,,,,8,,,,,,4,,,1,,,,,,,,,,,,,,,,54,12
- java.net,10,3,7,,,,,,,,,,,,,,10,,,,,,,,,,,,,,,,,,,,3,7,
+ java.net,10,3,7,,,,,,,,,,,,,,10,,,,,,,,,,,,,,,,,,,,,3,7,
- java.nio,15,,14,,13,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,14,
+ java.nio,15,,14,,13,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,14,
- java.sql,11,,,,,,,,,4,,,,,,,,,,,,,,,,7,,,,,,,,,,,,,
+ java.sql,11,,,,,,,,,4,,,,,,,,,,,,,,,,,7,,,,,,,,,,,,,
- java.util,44,,461,,,,,,,,,,,34,,,,,,5,2,,1,2,,,,,,,,,,,,,,36,425
+ java.util,44,,461,,,,,,,,,,,34,,,,,,,5,2,,1,2,,,,,,,,,,,,,,36,425
- javax.faces.context,2,7,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,7,,
+ javax.faces.context,2,7,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,7,,
- javax.jms,,9,57,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,9,57,
+ javax.jms,,9,57,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,9,57,
- javax.json,,,123,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,100,23
+ javax.json,,,123,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,100,23
- javax.management.remote,2,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ javax.management.remote,2,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- javax.naming,7,,,,,,,,,,,6,1,,,,,,,,,,,,,,,,,,,,,,,,,,
+ javax.naming,7,,,,,,,,,,,6,1,,,,,,,,,,,,,,,,,,,,,,,,,,,
- javax.net.ssl,2,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,,,,,,
+ javax.net.ssl,2,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,,,,,,
- javax.script,1,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,
+ javax.script,1,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,
- javax.servlet,4,21,2,,,,3,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,21,2,
+ javax.servlet,4,21,2,,,,3,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,21,2,
- javax.validation,1,1,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,
+ javax.validation,1,1,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,
- javax.ws.rs.client,1,,,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,
+ javax.ws.rs.client,1,,,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,
- javax.ws.rs.container,,9,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,9,,
+ javax.ws.rs.container,,9,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,9,,
- javax.ws.rs.core,3,,149,,,,1,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,94,55
+ javax.ws.rs.core,3,,149,,,,1,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,94,55
- javax.xml.transform,1,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,,,,6,
+ javax.xml.transform,1,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,,,,6,
- javax.xml.xpath,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,,,,,,,,
+ javax.xml.xpath,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,,,,,,,,
- jodd.json,,,10,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,10
+ jodd.json,,,10,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,10
- kotlin,12,,1835,,10,,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,1828,7
+ kotlin,12,,1835,,10,,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,,1828,7
- net.sf.saxon.s9api,5,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,5,,,,,,,
+ net.sf.saxon.s9api,5,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,5,,,,,,,
- ognl,6,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,
+ ognl,6,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,,
- okhttp3,2,,47,,,,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,22,25
+ okhttp3,2,,47,,,,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,,22,25
- org.apache.commons.codec,,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,
+ org.apache.commons.codec,,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,
- org.apache.commons.collections,,,800,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,17,783
+ org.apache.commons.collections,,,800,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,17,783
- org.apache.commons.collections4,,,800,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,17,783
+ org.apache.commons.collections4,,,800,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,17,783
- org.apache.commons.io,106,,556,,91,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,,542,14
+ org.apache.commons.io,106,,556,,91,,,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,,,542,14
- org.apache.commons.jexl2,15,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.apache.commons.jexl2,15,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.apache.commons.jexl3,15,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.apache.commons.jexl3,15,,,,,,,,,,15,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.apache.commons.lang3,,,424,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,293,131
+ org.apache.commons.lang3,6,,424,,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,293,131
- org.apache.commons.logging,6,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.apache.commons.logging,6,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.apache.commons.ognl,6,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,
+ org.apache.commons.ognl,6,,,,,,,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,,
- org.apache.commons.text,,,272,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,220,52
+ org.apache.commons.text,,,272,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,220,52
- org.apache.directory.ldap.client.api,1,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.apache.directory.ldap.client.api,1,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.apache.hc.core5.function,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
+ org.apache.hc.core5.function,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
- org.apache.hc.core5.http,1,2,39,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,,2,39,
+ org.apache.hc.core5.http,1,2,39,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,,2,39,
- org.apache.hc.core5.net,,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,
+ org.apache.hc.core5.net,,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,
- org.apache.hc.core5.util,,,24,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,18,6
+ org.apache.hc.core5.util,,,24,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,18,6
- org.apache.http,27,3,70,,,,,,,,,,,,,,25,,,,,,,,,,,,,,,,2,,,,3,62,8
+ org.apache.http,27,3,70,,,,,,,,,,,,,,25,,,,,,,,,,,,,,,,,2,,,,3,62,8
- org.apache.ibatis.jdbc,6,,57,,,,,,,,,,,,,,,,,,,,,,,6,,,,,,,,,,,,57,
+ org.apache.ibatis.jdbc,6,,57,,,,,,,,,,,,,,,,,,,,,,,,6,,,,,,,,,,,,57,
- org.apache.log4j,11,,,,,,,,,,,,,11,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.apache.log4j,11,,,,,,,,,,,,,11,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.apache.logging.log4j,359,,8,,,,,,,,,,,359,,,,,,,,,,,,,,,,,,,,,,,,4,4
+ org.apache.logging.log4j,359,,8,,,,,,,,,,,359,,,,,,,,,,,,,,,,,,,,,,,,,4,4
- org.apache.shiro.codec,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
+ org.apache.shiro.codec,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
- org.apache.shiro.jndi,1,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.apache.shiro.jndi,1,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.apache.velocity.app,4,,,,,,,,,,,,,,,,,,,,,,,,,,4,,,,,,,,,,,,
+ org.apache.velocity.app,4,,,,,,,,,,,,,,,,,,,,,,,,,,,4,,,,,,,,,,,,
- org.apache.velocity.runtime,4,,,,,,,,,,,,,,,,,,,,,,,,,,4,,,,,,,,,,,,
+ org.apache.velocity.runtime,4,,,,,,,,,,,,,,,,,,,,,,,,,,,4,,,,,,,,,,,,
- org.codehaus.groovy.control,1,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.codehaus.groovy.control,1,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.dom4j,20,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,20,,,,,,,,
+ org.dom4j,20,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,20,,,,,,,,
- org.hibernate,7,,,,,,,,,,,,,,,,,,,,,,,,,7,,,,,,,,,,,,,
+ org.hibernate,7,,,,,,,,,,,,,,,,,,,,,,,,,,7,,,,,,,,,,,,,
- org.jboss.logging,324,,,,,,,,,,,,,324,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.jboss.logging,324,,,,,,,,,,,,,324,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.jdbi.v3.core,6,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.jdbi.v3.core,6,,,,,,,,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.jooq,1,,,,,,,,,,,,,,,,,,,,,,,,,1,,,,,,,,,,,,,
+ org.jooq,1,,,,,,,,,,,,,,,,,,,,,,,,,,1,,,,,,,,,,,,,
- org.json,,,236,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,198,38
+ org.json,,,236,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,198,38
- org.mvel2,16,,,,,,,,,,,,,,16,,,,,,,,,,,,,,,,,,,,,,,,
+ org.mvel2,16,,,,,,,,,,,,,,16,,,,,,,,,,,,,,,,,,,,,,,,,
- org.scijava.log,13,,,,,,,,,,,,,13,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.scijava.log,13,,,,,,,,,,,,,13,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.slf4j,55,,6,,,,,,,,,,,55,,,,,,,,,,,,,,,,,,,,,,,,2,4
+ org.slf4j,55,,6,,,,,,,,,,,55,,,,,,,,,,,,,,,,,,,,,,,,,2,4
- org.springframework.beans,,,30,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,30
+ org.springframework.beans,,,30,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,30
- org.springframework.boot.jdbc,1,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.springframework.boot.jdbc,1,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.springframework.cache,,,13,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,13
+ org.springframework.cache,,,13,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,13
- org.springframework.context,,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,
+ org.springframework.context,,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,
- org.springframework.data.repository,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1
+ org.springframework.data.repository,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1
- org.springframework.http,14,,70,,,,,,,,,,,,,,14,,,,,,,,,,,,,,,,,,,,,60,10
+ org.springframework.http,14,,70,,,,,,,,,,,,,,14,,,,,,,,,,,,,,,,,,,,,,60,10
- org.springframework.jdbc.core,10,,,,,,,,,,,,,,,,,,,,,,,,,10,,,,,,,,,,,,,
+ org.springframework.jdbc.core,10,,,,,,,,,,,,,,,,,,,,,,,,,,10,,,,,,,,,,,,,
- org.springframework.jdbc.datasource,4,,,,,,,,,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.springframework.jdbc.datasource,4,,,,,,,,,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.springframework.jdbc.object,9,,,,,,,,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,,,
+ org.springframework.jdbc.object,9,,,,,,,,,,,,,,,,,,,,,,,,,,9,,,,,,,,,,,,,
- org.springframework.jndi,1,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.springframework.jndi,1,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.springframework.ldap,47,,,,,,,,,,,33,14,,,,,,,,,,,,,,,,,,,,,,,,,,
+ org.springframework.ldap,47,,,,,,,,,,,33,14,,,,,,,,,,,,,,,,,,,,,,,,,,,
- org.springframework.security.web.savedrequest,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,,
+ org.springframework.security.web.savedrequest,,6,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,,
- org.springframework.ui,,,32,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,32
+ org.springframework.ui,,,32,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,32
- org.springframework.util,,,139,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,87,52
+ org.springframework.util,,,139,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,87,52
- org.springframework.validation,,,13,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,13,
+ org.springframework.validation,,,13,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,13,
- org.springframework.web.client,13,3,,,,,,,,,,,,,,,13,,,,,,,,,,,,,,,,,,,,3,,
+ org.springframework.web.client,13,3,,,,,,,,,,,,,,,13,,,,,,,,,,,,,,,,,,,,,3,,
- org.springframework.web.context.request,,8,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,8,,
+ org.springframework.web.context.request,,8,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,8,,
- org.springframework.web.multipart,,12,13,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,12,13,
+ org.springframework.web.multipart,,12,13,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,12,13,
- org.springframework.web.reactive.function.client,2,,,,,,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,,
+ org.springframework.web.reactive.function.client,2,,,,,,,,,,,,,,,,2,,,,,,,,,,,,,,,,,,,,,,,
- org.springframework.web.util,,,163,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,138,25
+ org.springframework.web.util,,,163,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,138,25
- org.thymeleaf,2,,2,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,,,2,
+ org.thymeleaf,2,,2,,,,,,,,,,,,,,,,,,,,,,,,,2,,,,,,,,,,,2,
- org.xml.sax,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
+ org.xml.sax,,,1,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,1,
- org.xmlpull.v1,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,,
+ org.xmlpull.v1,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,,
- play.mvc,,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,4,,
+ play.mvc,,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,4,,
- ratpack.core.form,,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,
+ ratpack.core.form,,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,
- ratpack.core.handling,,6,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,4,
+ ratpack.core.handling,,6,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,4,
- ratpack.core.http,,10,10,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,10,10,
+ ratpack.core.http,,10,10,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,10,10,
- ratpack.exec,,,48,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,48
+ ratpack.exec,,,48,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,48
- ratpack.form,,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,
+ ratpack.form,,,3,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,3,
- ratpack.func,,,35,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,35
+ ratpack.func,,,35,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,35
- ratpack.handling,,6,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,4,
+ ratpack.handling,,6,4,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,6,4,
- ratpack.http,,10,10,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,10,10,
+ ratpack.http,,10,10,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,10,10,
- ratpack.util,,,35,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,35
+ ratpack.util,,,35,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,35
- retrofit2,1,,,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,
+ retrofit2,1,,,,,,,,,,,,,,,,1,,,,,,,,,,,,,,,,,,,,,,,

atorralba

Mostly LGTM, with a couple of minor comments. Curious about why exactly we couldn't use the regex-use[*] format for the new sinks so that the existing ReDoS queries could benefit from them.

atorralba · 2022-11-07T08:50:30Z

java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll

+    exists(string kind |
+      kind.matches(["regex-use[]", "regex-use[f1]", "regex-use[f-1]", "regex-use[-1]", "regex-use"]) and
+      sinkNode(this, kind)
+    )


I you're not using wildcards (still curious why regex-use% doesn't work), you don't need matches (and then the exists isn't really needed either):

Suggested change

exists(string kind |

kind.matches(["regex-use[]", "regex-use[f1]", "regex-use[f-1]", "regex-use[-1]", "regex-use"]) and

sinkNode(this, kind)

)

sinkNode(this, ["regex-use[]", "regex-use[f1]", "regex-use[f-1]", "regex-use[-1]", "regex-use"])

Some of the regex-use sinks are specific to the ReDoS query; in particular the Predicate.test sink is likely to cause many false positives if included. Which is probably the reason regex-use[0] is being excluded here.

The best solution is probably to exclude that sink specifically - perhaps by giving it a seperate kind that only the ReDoS query checks for. (which it does so here)

Ah, makes sense, thanks! I would be surprised if more queries used these sinks aside from ReDos and regex injection, but if you think it makes sense we can create a special sink kind for Predicate.test. Specially if regex-use[0] includes other sinks that are valid for this query.

atorralba · 2022-11-07T08:50:30Z

java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll

+    exists(MethodAccess ma, Method m, Field field | m = ma.getMethod() |
+      ma.getArgument(0) = this.asExpr() and
+      m.getDeclaringType() instanceof TypeRegexPattern and
+      m.hasName("compile") and
+      field instanceof PatternLiteralField and
+      ma.getArgument(1) = field.getAnAccess()


You can remove the instanceof by using the stricter type directly in exists:

Suggested change

exists(MethodAccess ma, Method m, Field field | m = ma.getMethod() |

ma.getArgument(0) = this.asExpr() and

m.getDeclaringType() instanceof TypeRegexPattern and

m.hasName("compile") and

field instanceof PatternLiteralField and

ma.getArgument(1) = field.getAnAccess()

exists(MethodAccess ma, Method m, PatternLiteralField field | m = ma.getMethod() |

ma.getArgument(0) = this.asExpr() and

m.getDeclaringType() instanceof TypeRegexPattern and

m.hasName("compile") and

ma.getArgument(1) = field.getAnAccess()

atorralba · 2022-11-07T08:50:30Z

java/ql/test/query-tests/security/CWE-730/RegexInjectionTest.ql

+import TestUtilities.InlineExpectationsTest
+import semmle.code.java.security.regexp.RegexInjectionQuery
+
+//import semmle.code.java.security.regexp.PolynomialReDoSQuery


This can be removed.

Suggested change

//import semmle.code.java.security.regexp.PolynomialReDoSQuery

Jami Cogswell added 14 commits Nov 1, 2022

move files out of experimental

d95da73

update options and qlref files

3925a3e

move to .qll file and switch to InlineExpectations tests

520ac01

add Pattern.quote sanitizer

cbc71fb

adjust imports

29d84b5

add classes for Pattern, Matcher, and RegExUtils

3e67d05

refactor code; add change note

a76c9a6

create RegexInjection.qll file

6ce2c90

refactor into more classes; add more test cases; add LITERAL sanitizer

8713223

split sanitizer into three

e392843

update sink names

0ff10ad

move Pattern.quote and Pattern.LITERAL models to Regex.qll

863a2b9

update help file

c5db9e3

clean up files

b19b39a

github-actions bot added documentation Java labels Nov 1, 2022

github-code-scanning bot found potential problems Nov 1, 2022

View changes

java/ql/lib/semmle/code/java/frameworks/Regex.qll Fixed Show fixed Hide fixed

switch sink to use csv models

6b4a4f1

remove original sanitizer

b7316b7

Jami Cogswell added 3 commits Nov 3, 2022

move files to regexp directory

809184c

update tests

5258961

update change note

a1fd284

update sink and tests

122f624

jcogs33 marked this pull request as ready for review Nov 7, 2022

jcogs33 requested a review from a team as a code owner Nov 7, 2022

atorralba reviewed Nov 7, 2022

View changes

Java: Promote regex injection query from experimental #11070

Java: Promote regex injection query from experimental #11070

jcogs33 commented Nov 1, 2022

github-actions bot commented Nov 1, 2022

Regular expression injection

Recommendation

Example

References

github-actions bot commented Nov 3, 2022

java

Generated file changes for java

github-actions bot commented Nov 3, 2022

java

Generated file changes for java

github-actions bot commented Nov 3, 2022

java

Generated file changes for java

github-actions bot commented Nov 4, 2022

java

Generated file changes for java

atorralba left a comment

atorralba Nov 7, 2022

joefarebrother Nov 7, 2022 •

edited

atorralba Nov 7, 2022

atorralba Nov 7, 2022

atorralba Nov 7, 2022

Java: Promote regex injection query from experimental #11070

Are you sure you want to change the base?

Java: Promote regex injection query from experimental #11070

Conversation

jcogs33 commented Nov 1, 2022

github-actions bot commented Nov 1, 2022

Regular expression injection

Recommendation

Example

References

github-actions bot commented Nov 3, 2022

java

Generated file changes for java

github-actions bot commented Nov 3, 2022

java

Generated file changes for java

github-actions bot commented Nov 3, 2022

java

Generated file changes for java

github-actions bot commented Nov 4, 2022

java

Generated file changes for java

atorralba left a comment

atorralba Nov 7, 2022

Choose a reason for hiding this comment

joefarebrother Nov 7, 2022 • edited

Choose a reason for hiding this comment

atorralba Nov 7, 2022

Choose a reason for hiding this comment

atorralba Nov 7, 2022

Choose a reason for hiding this comment

atorralba Nov 7, 2022

Choose a reason for hiding this comment

joefarebrother Nov 7, 2022 •

edited