Skip to content

Commit ebecc19

Browse files
authored
Remove duplicated ReDoS fast-path filter from RegexGrammar; narrow at RegexFlowConfigs wrappers
1 parent 645f2fd commit ebecc19

2 files changed

Lines changed: 39 additions & 50 deletions

File tree

cpp/ql/lib/semmle/code/cpp/regex/RegexFlowConfigs.qll

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,31 +34,50 @@ import semmle.code.cpp.regex.internal.StdRegex
3434

3535
// ---------------------------------------------------------------------------
3636
// Re-exports from the flow-free grammar/flag module.
37+
//
3738
// These are kept in scope for backward compatibility so that consumers can
38-
// continue to import them from `RegexFlowConfigs`.
39+
// continue to import them from `RegexFlowConfigs`. They are narrowed here
40+
// to `ExploitableStringLiteral` — the same source set as the taint-tracking
41+
// configuration below — so the flag/grammar readers exposed at this layer
42+
// only report on regexes that are ReDoS-analysis candidates. The
43+
// underlying flow-free predicates in `RegexGrammar` are unrestricted; any
44+
// consumer wanting the full set can call them directly.
3945
// ---------------------------------------------------------------------------
4046
/** See `RegexGrammar::TRegexGrammar`. */
4147
class TRegexGrammar = RG::TRegexGrammar;
4248

4349
/** See `RegexGrammar::regexGrammar`. */
44-
TRegexGrammar regexGrammar(StringLiteral regex) { result = RG::regexGrammar(regex) }
50+
TRegexGrammar regexGrammar(StringLiteral regex) {
51+
regex instanceof ExploitableStringLiteral and
52+
result = RG::regexGrammar(regex)
53+
}
4554

4655
/** See `RegexGrammar::hasConcreteGrammar`. */
4756
predicate hasConcreteGrammar(TRegexGrammar grammar) { RG::hasConcreteGrammar(grammar) }
4857

4958
/** See `RegexGrammar::hasIgnoreCaseFlag`. */
50-
predicate hasIgnoreCaseFlag(StringLiteral regex) { RG::hasIgnoreCaseFlag(regex) }
59+
predicate hasIgnoreCaseFlag(StringLiteral regex) {
60+
regex instanceof ExploitableStringLiteral and
61+
RG::hasIgnoreCaseFlag(regex)
62+
}
5163

5264
/** See `RegexGrammar::hasMultilineFlag`. */
53-
predicate hasMultilineFlag(StringLiteral regex) { RG::hasMultilineFlag(regex) }
65+
predicate hasMultilineFlag(StringLiteral regex) {
66+
regex instanceof ExploitableStringLiteral and
67+
RG::hasMultilineFlag(regex)
68+
}
5469

5570
/** See `RegexGrammar::hasNonEcmaScriptGrammarFlag`. */
5671
predicate hasNonEcmaScriptGrammarFlag(StringLiteral regex) {
72+
regex instanceof ExploitableStringLiteral and
5773
RG::hasNonEcmaScriptGrammarFlag(regex)
5874
}
5975

6076
/** See `RegexGrammar::hasEcmaScriptGrammarFlag`. */
61-
predicate hasEcmaScriptGrammarFlag(StringLiteral regex) { RG::hasEcmaScriptGrammarFlag(regex) }
77+
predicate hasEcmaScriptGrammarFlag(StringLiteral regex) {
78+
regex instanceof ExploitableStringLiteral and
79+
RG::hasEcmaScriptGrammarFlag(regex)
80+
}
6281

6382
// ---------------------------------------------------------------------------
6483
// std::basic_regex type helpers

cpp/ql/lib/semmle/code/cpp/regex/internal/RegexGrammar.qll

Lines changed: 15 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -23,27 +23,6 @@
2323
import cpp
2424
private import semmle.code.cpp.regex.internal.StdRegex
2525

26-
// ---------------------------------------------------------------------------
27-
// Fast-path filter: only consider literals that *look* regex-y
28-
// ---------------------------------------------------------------------------
29-
/**
30-
* Holds if `s` is a plausible ReDoS-candidate string literal value: it
31-
* contains at least one unbounded-repetition quantifier (`+`, `*`, or
32-
* `{n,}`).
33-
*
34-
* This mirrors the source restriction of the dataflow configuration in
35-
* `RegexFlowConfigs` (whose sources are the `ExploitableStringLiteral`s),
36-
* so that the flag readers below consider exactly the same set of regexes
37-
* as the taint-tracking configuration. Inlined here as a syntactic check
38-
* to keep this module flow-free.
39-
*/
40-
private predicate isExploitableStringLiteralValue(StringLiteral s) {
41-
exists(string v | v = s.getValue() |
42-
v.regexpMatch(".*[+*].*") or
43-
v.regexpMatch(".*\\{[0-9]+,[0-9]*\\}.*")
44-
)
45-
}
46-
4726
// ---------------------------------------------------------------------------
4827
// Construction-site flag readers (std::regex_constants enum plumbing)
4928
// ---------------------------------------------------------------------------
@@ -108,32 +87,23 @@ private predicate isPatternLiteralOf(StringLiteral regex, Expr patternArg) {
10887
*
10988
* This association is purely syntactic (no dataflow): the pattern literal
11089
* must appear inside the constructor/assign call's first argument.
111-
*
112-
* Restricted to string literals that look like plausible ReDoS candidates
113-
* (see `isExploitableStringLiteralValue`) to preserve the exact set of
114-
* regexes considered by the flag readers when they were routed through
115-
* the taint-tracking configuration (whose sources are the same
116-
* `ExploitableStringLiteral` set).
11790
*/
11891
private Expr getConstructionFlagArg(StringLiteral regex) {
119-
isExploitableStringLiteralValue(regex) and
120-
(
121-
// basic_regex(pattern, flags) constructor - both named-variable and
122-
// temporary constructions are covered because the search for `regex`
123-
// is done syntactically inside the constructor's first argument.
124-
exists(ConstructorCall cc |
125-
cc.getTarget().getDeclaringType() instanceof StdBasicRegex and
126-
isPatternLiteralOf(regex, cc.getArgument(0)) and
127-
result = cc.getArgument(1)
128-
)
129-
or
130-
// basic_regex::assign(pattern, flags).
131-
exists(FunctionCall fc |
132-
fc.getTarget().(MemberFunction).getDeclaringType() instanceof StdBasicRegex and
133-
fc.getTarget().hasName("assign") and
134-
isPatternLiteralOf(regex, fc.getArgument(0)) and
135-
result = fc.getArgument(1)
136-
)
92+
// basic_regex(pattern, flags) constructor - both named-variable and
93+
// temporary constructions are covered because the search for `regex`
94+
// is done syntactically inside the constructor's first argument.
95+
exists(ConstructorCall cc |
96+
cc.getTarget().getDeclaringType() instanceof StdBasicRegex and
97+
isPatternLiteralOf(regex, cc.getArgument(0)) and
98+
result = cc.getArgument(1)
99+
)
100+
or
101+
// basic_regex::assign(pattern, flags).
102+
exists(FunctionCall fc |
103+
fc.getTarget().(MemberFunction).getDeclaringType() instanceof StdBasicRegex and
104+
fc.getTarget().hasName("assign") and
105+
isPatternLiteralOf(regex, fc.getArgument(0)) and
106+
result = fc.getArgument(1)
137107
)
138108
}
139109

0 commit comments

Comments
 (0)