mirror of
https://github.com/github/codeql.git
synced 2026-05-14 11:19:27 +02:00
Refactor regex-use sink kind to support polynomial ReDoS query
- Extended regexSinkKindInfo to handle plain "regex-use" sink kind - Plain "regex-use" defaults to non-full-string matching with string arg at index 0 - Updated comment in org.apache.commons.lang3.model.yml - Added test cases for RegExUtils methods in polynomial ReDoS test Co-authored-by: owen-mc <62447351+owen-mc@users.noreply.github.com>
This commit is contained in:
@@ -4,9 +4,9 @@ extensions:
|
||||
extensible: sinkModel
|
||||
data:
|
||||
- ["org.apache.commons.lang3", "SerializationUtils", False, "deserialize", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
|
||||
# Note these sinks do not use the sink kind `regex-use[0]` because the regex injection query needs to select them separately from
|
||||
# other `regex-use[0]` sinks in order to avoid FPs. As a result, these sinks are currently not used in the polynomial ReDoS query.
|
||||
# TODO: refactor the `regex-use%` sink kind so that the polynomial ReDoS query can also use these sinks.
|
||||
# Note these sinks use the sink kind `regex-use` (without brackets) because the regex injection query needs to select them
|
||||
# separately from other `regex-use[0]` sinks in order to avoid FPs. The polynomial ReDoS query handles the plain `regex-use`
|
||||
# sink kind by treating it as `regex-use[0]` (non-full-string matching with the string argument at index 0).
|
||||
- ["org.apache.commons.lang3", "RegExUtils", False, "removeAll", "(String,String)", "", "Argument[1]", "regex-use", "manual"]
|
||||
- ["org.apache.commons.lang3", "RegExUtils", False, "removeFirst", "(String,String)", "", "Argument[1]", "regex-use", "manual"]
|
||||
- ["org.apache.commons.lang3", "RegExUtils", False, "removePattern", "(String,String)", "", "Argument[1]", "regex-use", "manual"]
|
||||
|
||||
@@ -20,29 +20,32 @@ private class ExploitableStringLiteral extends StringLiteral {
|
||||
* `strArg` is the index of the argument to methods with this sink kind that
|
||||
* contain the string to be matched against, where -1 is the qualifier; or -2
|
||||
* if no such argument exists.
|
||||
*
|
||||
* Note that `regex-use` is deliberately not a possible value for `kind` here,
|
||||
* as it is used for regular expression injection sinks that need to be selected
|
||||
* separately from existing `regex-use[0]` sinks.
|
||||
* TODO: refactor the `regex-use%` sink kind so that the polynomial ReDoS query
|
||||
* can also use the `regex-use` sinks.
|
||||
*/
|
||||
private predicate regexSinkKindInfo(string kind, boolean full, int strArg) {
|
||||
sinkModel(_, _, _, _, _, _, _, kind, _, _) and
|
||||
exists(string fullStr, string strArgStr |
|
||||
(
|
||||
full = true and fullStr = "f"
|
||||
or
|
||||
full = false and fullStr = ""
|
||||
) and
|
||||
(
|
||||
strArgStr.toInt() = strArg
|
||||
or
|
||||
strArg = -2 and
|
||||
strArgStr = ""
|
||||
(
|
||||
exists(string fullStr, string strArgStr |
|
||||
(
|
||||
full = true and fullStr = "f"
|
||||
or
|
||||
full = false and fullStr = ""
|
||||
) and
|
||||
(
|
||||
strArgStr.toInt() = strArg
|
||||
or
|
||||
strArg = -2 and
|
||||
strArgStr = ""
|
||||
)
|
||||
|
|
||||
kind = "regex-use[" + fullStr + strArgStr + "]"
|
||||
)
|
||||
|
|
||||
kind = "regex-use[" + fullStr + strArgStr + "]"
|
||||
or
|
||||
// Plain `regex-use` sinks default to non-full-string matching with the string argument at index 0.
|
||||
// This is primarily used for methods like `RegExUtils.removeAll(text, regex)` where both the
|
||||
// pattern and the input are provided in the same call.
|
||||
kind = "regex-use" and
|
||||
full = false and
|
||||
strArg = 0
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import java.util.regex.Pattern;
|
||||
import java.util.function.Predicate;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import com.google.common.base.Splitter;
|
||||
import org.apache.commons.lang3.RegExUtils;
|
||||
|
||||
class PolyRedosTest {
|
||||
void test(HttpServletRequest request) {
|
||||
@@ -81,4 +82,16 @@ class PolyRedosTest {
|
||||
p.matcher(request.getRequestURI()).matches();
|
||||
p.matcher(request.getCookies()[0].getName()).matches();
|
||||
}
|
||||
|
||||
void test7(HttpServletRequest request) {
|
||||
String tainted = request.getParameter("inp"); // $ Source
|
||||
String reg = "0\\.\\d+E?\\d+!";
|
||||
|
||||
RegExUtils.removeAll(tainted, reg); // $ Alert
|
||||
RegExUtils.removeFirst(tainted, reg); // $ Alert
|
||||
RegExUtils.removePattern(tainted, reg); // $ Alert
|
||||
RegExUtils.replaceAll(tainted, reg, "replacement"); // $ Alert
|
||||
RegExUtils.replaceFirst(tainted, reg, "replacement"); // $ Alert
|
||||
RegExUtils.replacePattern(tainted, reg, "replacement"); // $ Alert
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user