This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch bound-matchesregex-condition-evaluation in repository https://gitbox.apache.org/repos/asf/unomi.git
commit a69e2be6781531a7df6bd6f9181e4e566ad49ea8 Author: Serge Huber <[email protected]> AuthorDate: Tue Aug 18 21:51:30 2026 +0200 Bound regular-expression evaluation in the matchesRegex operator The matchesRegex condition operator compiled and evaluated a caller-supplied regular expression with no bound on the work performed, so a pathological pattern could consume a request-handling thread. Route it through a guarded evaluation that caps the pattern and input lengths, caches compiled patterns in a bounded LRU, and aborts once a fixed character-access budget is exceeded. All limits are configurable via org.apache.unomi.conditions.regex.* system properties, and regex semantics are unchanged for well-behaved patterns. Add tests covering normal matches, invalid patterns, catastrophic backtracking, and oversized pattern/input rejection. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../conditions/PropertyConditionEvaluator.java | 98 +++++++++++++++++++++- .../conditions/PropertyConditionEvaluatorTest.java | 41 +++++++++ 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java index 8229fe6c1..82d0a379a 100644 --- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java +++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java @@ -40,6 +40,7 @@ import org.slf4j.LoggerFactory; import java.text.SimpleDateFormat; import java.util.*; import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; import java.util.stream.Collectors; import static org.apache.unomi.persistence.spi.conditions.DateUtils.getDate; @@ -58,6 +59,28 @@ public class PropertyConditionEvaluator implements ConditionEvaluator { private static final HardcodedPropertyAccessorRegistry hardcodedPropertyAccessorRegistry = new HardcodedPropertyAccessorRegistry(); private ExpressionFilterFactory expressionFilterFactory; + // The matchesRegex operator compiles and evaluates a regular expression that can originate from an + // untrusted, unauthenticated caller (a condition tree on the public context/eventcollector endpoints, + // or a GraphQL filter). java.util.regex is a backtracking engine, so an unbounded evaluation is a + // single-request CPU-exhaustion primitive. These guards bound the pattern length, the matched value + // length, and the total work the matcher may perform. All limits are overridable via system property + // for the rare deployment with legitimate larger needs. + private static final int MAX_REGEX_PATTERN_LENGTH = + Integer.getInteger("org.apache.unomi.conditions.regex.maxPatternLength", 512); + private static final int MAX_REGEX_INPUT_LENGTH = + Integer.getInteger("org.apache.unomi.conditions.regex.maxInputLength", 10000); + private static final long MAX_REGEX_CHAR_ACCESSES = + Long.getLong("org.apache.unomi.conditions.regex.maxCharAccesses", 1000000L); + private static final int MAX_CACHED_REGEX_PATTERNS = 1000; + + private static final Map<String, Pattern> compiledRegexCache = + Collections.synchronizedMap(new LinkedHashMap<String, Pattern>(16, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry<String, Pattern> eldest) { + return size() > MAX_CACHED_REGEX_PATTERNS; + } + }); + private TracerService tracerService; public void setUsePropertyConditionOptimizations(boolean usePropertyConditionOptimizations) { @@ -441,7 +464,7 @@ public class PropertyConditionEvaluator implements ConditionEvaluator { } else if (op.equals("endsWith")) { return actualValue.toString().endsWith(expectedValue); } else if (op.equals("matchesRegex")) { - return expectedValue != null && Pattern.compile(expectedValue).matcher(actualValue.toString()).matches(); + return expectedValue != null && matchesRegexSafely(expectedValue, actualValue.toString()); } else if (op.equals("in") || op.equals("inContains") || op.equals("notIn") || op.equals("hasSomeOf") || op.equals("hasNoneOf") || op.equals("all")) { Collection<?> expectedValues = ConditionContextHelper.foldToASCII((Collection<?>) condition.getParameter("propertyValues")); Collection<?> expectedValuesInteger = (Collection<?>) condition.getParameter("propertyValuesInteger"); @@ -485,6 +508,79 @@ public class PropertyConditionEvaluator implements ConditionEvaluator { return false; } + /** + * Evaluates the {@code matchesRegex} operator with ReDoS protections: the pattern and the matched + * value are length-capped, compiled patterns are cached, and the match runs against a budget-limited + * {@link CharSequence} so that catastrophic backtracking is cut off instead of pinning a request + * thread. Any guard violation or invalid pattern evaluates to {@code false} (the condition does not + * match) and is logged. Regex semantics are otherwise unchanged, so legitimate patterns behave as before. + */ + protected static boolean matchesRegexSafely(String regex, String value) { + if (regex.length() > MAX_REGEX_PATTERN_LENGTH) { + LOGGER.warn("matchesRegex pattern longer than {} characters rejected", MAX_REGEX_PATTERN_LENGTH); + return false; + } + if (value.length() > MAX_REGEX_INPUT_LENGTH) { + LOGGER.warn("matchesRegex evaluation skipped: value longer than {} characters", MAX_REGEX_INPUT_LENGTH); + return false; + } + try { + Pattern pattern = compiledRegexCache.computeIfAbsent(regex, Pattern::compile); + return pattern.matcher(new BudgetedCharSequence(value, new long[]{MAX_REGEX_CHAR_ACCESSES})).matches(); + } catch (PatternSyntaxException e) { + LOGGER.warn("matchesRegex evaluation skipped: invalid pattern: {}", e.getMessage()); + return false; + } catch (RegexBudgetExceededException e) { + LOGGER.warn("matchesRegex evaluation aborted after {} character accesses (possible ReDoS pattern)", + MAX_REGEX_CHAR_ACCESSES); + return false; + } + } + + private static final class RegexBudgetExceededException extends RuntimeException { + RegexBudgetExceededException() { + super(null, null, false, false); + } + } + + /** + * A {@link CharSequence} wrapper that throws once a shared budget of {@code charAt} accesses is + * exhausted, bounding the total work a backtracking regex engine can perform on it. The budget is a + * single-element array so it is shared across the sub-sequences the engine may create. + */ + private static final class BudgetedCharSequence implements CharSequence { + private final CharSequence delegate; + private final long[] budget; + + BudgetedCharSequence(CharSequence delegate, long[] budget) { + this.delegate = delegate; + this.budget = budget; + } + + @Override + public char charAt(int index) { + if (--budget[0] < 0) { + throw new RegexBudgetExceededException(); + } + return delegate.charAt(index); + } + + @Override + public int length() { + return delegate.length(); + } + + @Override + public CharSequence subSequence(int start, int end) { + return new BudgetedCharSequence(delegate.subSequence(start, end), budget); + } + + @Override + public String toString() { + return delegate.toString(); + } + } + protected Object getPropertyValue(Item item, String expression) throws Exception { if (usePropertyConditionOptimizations) { Object result = getHardcodedPropertyValue(item, expression); diff --git a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java index dd29d597f..0fee8b8a1 100644 --- a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java +++ b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java @@ -35,6 +35,7 @@ import java.util.regex.Pattern; import static junit.framework.TestCase.assertEquals; import static junit.framework.TestCase.assertNull; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; public class PropertyConditionEvaluatorTest { @@ -339,6 +340,46 @@ public class PropertyConditionEvaluatorTest { assertFalse("Non-date expected value should not match lessThanOrEqualTo comparison", result); } + @Test + public void testMatchesRegexStillMatchesNormalPatterns() { + assertTrue("Simple regex should still match", + propertyConditionEvaluator.isMatch("matchesRegex", "hello123", "[a-z]+\\d+", null, null, null, null, null)); + assertFalse("Simple regex should still not match", + propertyConditionEvaluator.isMatch("matchesRegex", "123hello", "[a-z]+\\d+", null, null, null, null, null)); + assertFalse("Invalid pattern should evaluate to false instead of throwing", + propertyConditionEvaluator.isMatch("matchesRegex", "anything", "([unclosed", null, null, null, null, null)); + } + + @Test(timeout = 5000) + public void testMatchesRegexCatastrophicBacktrackingIsBounded() { + // '(a+)+$' against a long run of 'a' followed by a non-matching character is a textbook + // exponential-backtracking (ReDoS) input; unbounded evaluation would run effectively forever. + StringBuilder subject = new StringBuilder(); + for (int i = 0; i < 100; i++) { + subject.append('a'); + } + subject.append('!'); + assertFalse("Catastrophic pattern must be aborted and evaluate to false", + propertyConditionEvaluator.isMatch("matchesRegex", subject.toString(), "(a+)+$", null, null, null, null, null)); + } + + @Test + public void testMatchesRegexOversizedInputsAreRejected() { + StringBuilder longPattern = new StringBuilder("^"); + for (int i = 0; i < 600; i++) { + longPattern.append('a'); + } + assertFalse("Over-long pattern must be rejected", + propertyConditionEvaluator.isMatch("matchesRegex", "aaa", longPattern.toString(), null, null, null, null, null)); + + StringBuilder longValue = new StringBuilder(); + for (int i = 0; i < 10001; i++) { + longValue.append('a'); + } + assertFalse("Over-long value must be rejected", + propertyConditionEvaluator.isMatch("matchesRegex", longValue.toString(), "a*", null, null, null, null, null)); + } + class HardcodedWorker implements Callable<Object> { @Override
