You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by js...@apache.org on 2026/09/28 10:24:04 UTC
(unomi) branch backport/bound-matchesregex-3.0.x created (now e4d513b9c)
This is an automated email from the ASF dual-hosted git repository.
jsinovassin pushed a change to branch backport/bound-matchesregex-3.0.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
at e4d513b9c Bound regular-expression evaluation in the matchesRegex operator (unomi-3.0.x)
This branch includes the following new commits:
new e4d513b9c Bound regular-expression evaluation in the matchesRegex operator (unomi-3.0.x)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(unomi) 01/01: Bound regular-expression evaluation in the matchesRegex operator (unomi-3.0.x)
Posted by js...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jsinovassin pushed a commit to branch backport/bound-matchesregex-3.0.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
commit e4d513b9c4a4450b44d069325c81407f27c70f01
Author: jsinovassin <js...@jahia.com>
AuthorDate: Mon Sep 28 12:23:51 2026 +0200
Bound regular-expression evaluation in the matchesRegex operator (unomi-3.0.x)
Backport of #872 to unomi-3.0.x, re-cut from the current branch head.
The matchesRegex operator compiles and evaluates a pattern that an unauthenticated caller supplies
on the public context endpoint, against a subject the same caller chooses. java.util.regex is a
backtracking engine, so one request can hold a thread. The operator now runs under guards that cap
the pattern length at 512 and the input length at 10000, cache compiled patterns in a bounded LRU,
and abort once a character-access budget of one million is exceeded. Each limit is overridable by
system property under org.apache.unomi.conditions.regex, and well-behaved patterns keep the same
semantics.
Differences from master:
- AllITs registers ConditionRegexSafetyIT alone; the other suites the master commit lists do not
exist on this line.
- ConditionRegexSafetyIT sends no API key, because the context endpoint is public here. That is the
surface an unbounded evaluation is reachable from.
- One test case is added on each side, in the unit suite and in the integration suite, for the
bounded-quantifier shape. Both suites otherwise probe with (a+)+$, which uses an unbounded
quantifier. Java 17 optimizes that shape, so it costs nothing and the case passes whether or not
the guard is present: reverting the operator to the unguarded call leaves it green. The added
case uses ^(a|a){0,60}z$ against 40 characters, which is not optimized and whose cost doubles
with each added character, and reverting the operator turns it red on its timeout.
Tests: PropertyConditionEvaluatorTest runs 15 cases and passes. Reverting the operator to
Pattern.compile(...).matcher(...).matches() turns 3 of them red, one of which is the case added
here.
---
.../test/java/org/apache/unomi/itests/AllITs.java | 1 +
.../unomi/itests/ConditionRegexSafetyIT.java | 133 +++++++++++++++++++++
.../conditions/PropertyConditionEvaluator.java | 99 ++++++++++++++-
.../conditions/PropertyConditionEvaluatorTest.java | 59 +++++++++
4 files changed, 291 insertions(+), 1 deletion(-)
diff --git a/itests/src/test/java/org/apache/unomi/itests/AllITs.java b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
index be8f596a8..f05d5631b 100644
--- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
+++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
@@ -53,6 +53,7 @@ import org.junit.runners.Suite.SuiteClasses;
ModifyConsentIT.class,
PatchIT.class,
ContextServletIT.class,
+ ConditionRegexSafetyIT.class,
RuleServiceIT.class,
PrivacyServiceIT.class,
GroovyActionsServiceIT.class,
diff --git a/itests/src/test/java/org/apache/unomi/itests/ConditionRegexSafetyIT.java b/itests/src/test/java/org/apache/unomi/itests/ConditionRegexSafetyIT.java
new file mode 100644
index 000000000..cfca1787f
--- /dev/null
+++ b/itests/src/test/java/org/apache/unomi/itests/ConditionRegexSafetyIT.java
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.unomi.itests;
+
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+
+/**
+ * A personalization condition on the public tracking endpoints lets the caller supply a regular
+ * expression outright, without administrator credentials. If its evaluation were unbounded, one small
+ * request would occupy a request thread indefinitely.
+ * <p>
+ * These tests assert the property that matters operationally — the request <em>returns</em> — rather
+ * than any particular status code. The inputs used are ones that took exponential time before the fix
+ * (seconds at 26 characters, and far beyond that at the lengths used here), so a regression does not
+ * produce a wrong answer, it produces a request that never comes back. Each is therefore bounded by a
+ * timeout: if the boundary regresses, the test fails by timing out rather than by assertion.
+ */
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class ConditionRegexSafetyIT extends BaseIT {
+
+ private final static String CONTEXT_URL = "/cxs/context.json";
+
+ /** Generous next to the sub-millisecond a bounded evaluation needs, decisive against an unbounded one. */
+ private final static int RESPONSE_TIMEOUT_MS = 30_000;
+
+ /**
+ * A personalization filter carrying a caller-supplied pattern. {@code (a+)+$} against a long run of
+ * 'a' followed by a non-matching character is the textbook catastrophic case; the caller controls
+ * both the pattern and, through the identifier, much of the subject.
+ */
+ @Test(timeout = RESPONSE_TIMEOUT_MS)
+ public void testCallerSuppliedRegexConditionIsEvaluatedInBoundedTime() throws Exception {
+ final StringBuilder subject = new StringBuilder();
+ for (int i = 0; i < 60; i++) {
+ subject.append('a');
+ }
+
+ final String payload = "{\"sessionId\":\"regex-condition-session\",\"profileId\":\"" + subject + "\","
+ + "\"requiredProfileProperties\":[\"*\"],\"events\":[],"
+ + "\"filters\":[{\"id\":\"regex-safety-filter\",\"filters\":[{\"condition\":{"
+ + "\"type\":\"profilePropertyCondition\",\"parameterValues\":{"
+ + "\"propertyName\":\"itemId\",\"comparisonOperator\":\"matchesRegex\","
+ + "\"propertyValue\":\"(a+)+$\"}}}]}]}";
+
+ assertReturnsPromptly(payload);
+ }
+
+ /**
+ * The bounded-quantifier shape, which is the one that actually costs on a Java 17 engine.
+ * {@code (a+)+$} above uses an unbounded quantifier, and Java 17 optimizes that shape away, so
+ * the case above answers promptly whether or not the guard is present. {@code ^(a|a){0,60}z$}
+ * is not optimized: its cost doubles with each added character of the subject, and an unguarded
+ * evaluation of 40 characters runs for hours.
+ */
+ @Test(timeout = RESPONSE_TIMEOUT_MS)
+ public void testBoundedQuantifierRegexConditionIsEvaluatedInBoundedTime() throws Exception {
+ final StringBuilder subject = new StringBuilder();
+ for (int i = 0; i < 40; i++) {
+ subject.append('a');
+ }
+
+ final String payload = "{\"sessionId\":\"regex-bounded-session\",\"profileId\":\"" + subject + "\","
+ + "\"requiredProfileProperties\":[\"*\"],\"events\":[],"
+ + "\"filters\":[{\"id\":\"regex-bounded-filter\",\"filters\":[{\"condition\":{"
+ + "\"type\":\"profilePropertyCondition\",\"parameterValues\":{"
+ + "\"propertyName\":\"itemId\",\"comparisonOperator\":\"matchesRegex\","
+ + "\"propertyValue\":\"^(a|a){0,60}z$\"}}}]}]}";
+
+ assertReturnsPromptly(payload);
+ }
+
+ /** A pattern far longer than any legitimate one must be refused rather than compiled and run. */
+ @Test(timeout = RESPONSE_TIMEOUT_MS)
+ public void testOversizedCallerSuppliedRegexIsRefused() throws Exception {
+ final StringBuilder hugePattern = new StringBuilder();
+ for (int i = 0; i < 2000; i++) {
+ hugePattern.append("a?");
+ }
+
+ final String payload = "{\"sessionId\":\"regex-oversize-session\",\"requiredProfileProperties\":[\"*\"],"
+ + "\"events\":[],"
+ + "\"filters\":[{\"id\":\"regex-oversize-filter\",\"filters\":[{\"condition\":{"
+ + "\"type\":\"profilePropertyCondition\",\"parameterValues\":{"
+ + "\"propertyName\":\"itemId\",\"comparisonOperator\":\"matchesRegex\","
+ + "\"propertyValue\":\"" + hugePattern + "\"}}}]}]}";
+
+ assertReturnsPromptly(payload);
+ }
+
+ /**
+ * Sends the payload to the public context endpoint and requires an answer. Any HTTP status is
+ * acceptable — rejecting the input and processing it are both fine outcomes. What is not acceptable
+ * is the request not completing, which is exactly what an unbounded evaluation causes.
+ */
+ private void assertReturnsPromptly(final String payload) throws Exception {
+ final HttpPost request = new HttpPost(getFullUrl(CONTEXT_URL));
+ // No credential: on this line the context endpoint is public, which is the surface an
+ // unbounded evaluation is reachable from.
+ request.setEntity(new StringEntity(payload, ContentType.APPLICATION_JSON));
+
+ final long startedAt = System.currentTimeMillis();
+ try (CloseableHttpResponse response = httpClient.execute(request)) {
+ final long elapsed = System.currentTimeMillis() - startedAt;
+ Assert.assertNotNull("The public endpoint must answer rather than hang", response);
+ Assert.assertTrue("The public endpoint answered, but took " + elapsed
+ + "ms, which suggests the evaluation is no longer bounded", elapsed < RESPONSE_TIMEOUT_MS);
+ }
+ }
+}
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 e6c391fdf..2e245283f 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
@@ -38,6 +38,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;
@@ -56,6 +57,29 @@ 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;
+ }
+ });
+
+
public void setUsePropertyConditionOptimizations(boolean usePropertyConditionOptimizations) {
this.usePropertyConditionOptimizations = usePropertyConditionOptimizations;
}
@@ -256,7 +280,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");
@@ -300,6 +324,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 07bc4d2e8..64baea80e 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
@@ -32,6 +32,8 @@ 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 {
@@ -202,4 +204,61 @@ public class PropertyConditionEvaluatorTest {
}
}
+ @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));
+ }
+
+ /**
+ * The bounded-quantifier shape, which is the one that actually costs on a Java 17 engine.
+ * {@code (a+)+$} above uses an unbounded quantifier, and Java 17 optimizes that shape away, so
+ * that test passes whether or not the guard is present and cannot discriminate on its own.
+ * {@code ^(a|a){0,60}z$} is not optimized: its cost doubles with each added character of the
+ * subject, and an unguarded evaluation of 40 characters runs for hours.
+ */
+ @Test(timeout = 5000)
+ public void testMatchesRegexBoundedQuantifierIsAborted() {
+ StringBuilder subject = new StringBuilder();
+ for (int i = 0; i < 40; i++) {
+ subject.append('a');
+ }
+ assertFalse("Bounded-quantifier pattern must be aborted and evaluate to false",
+ propertyConditionEvaluator.isMatch("matchesRegex", subject.toString(), "^(a|a){0,60}z$", 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));
+ }
+
}