This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-jspf.git
commit 4db83ce515c2258d73d5ea13e716e4277dabf085 Author: Quan Tran <[email protected]> AuthorDate: Fri Oct 30 16:21:22 2026 +0700 [FIX] Domain-spec regexes: do not recurse once per character java.util.regex recurses once per iteration of a repeated group it may backtrack into. The macro-string of a domain-spec was such a group, repeated once per character, so a domain-spec of a few thousand characters (an include: target, an exp= or redirect= domain), or the value of an unknown modifier, overflowed the stack while parsing the record or expanding its macros, and the StackOverflowError escaped SPF.checkSPF. With a 256k thread stack, 218 characters were enough. Match the macro-expands of a macro-string with a possessive loop: a macro-literal is never "%", so a macro-string splits into its tokens one way only and there is nothing to backtrack. Then match the domain-end: macro-literals ending with "." toplabel, or a last macro-expand. The second toplabel alternative repeats a character class instead of a group. MacroExpand now uses the parser's domain-spec regex: the two groups of DOMAIN_SPEC_REGEX_R only filled a buffer that was never read. That constant and MACRO_STRING_REGEX keep their values, deprecated, for the code outside jSPF that builds patterns from them. An unknown modifier now ends with POSSESSIVE_MACRO_STRING_REGEX. DOMAIN_SPEC_REGEX matches the same domain-specs, but a pattern appended to it can no longer hold a macro-expand. --- .../org/apache/james/jspf/core/MacroExpand.java | 18 +------- .../apache/james/jspf/core/SPFTermsRegexps.java | 38 ++++++++++++++-- .../apache/james/jspf/terms/UnknownModifier.java | 2 +- .../org/apache/james/jspf/SPFExecutorBaseTest.java | 30 ++++++++++--- .../apache/james/jspf/core/MacroExpandTest.java | 14 ++++++ .../james/jspf/core/SPFTermsRegexpsTest.java | 50 ++++++++++++++++++++++ .../james/jspf/terms/UnknownModifierTest.java | 40 +++++++++++++++++ 7 files changed, 165 insertions(+), 27 deletions(-) diff --git a/resolver/src/main/java/org/apache/james/jspf/core/MacroExpand.java b/resolver/src/main/java/org/apache/james/jspf/core/MacroExpand.java index 4a882c8..3e9c5f1 100644 --- a/resolver/src/main/java/org/apache/james/jspf/core/MacroExpand.java +++ b/resolver/src/main/java/org/apache/james/jspf/core/MacroExpand.java @@ -73,8 +73,7 @@ public class MacroExpand { * @param dnsProbe the dns service to use */ public MacroExpand(DNSService dnsProbe) { - // This matches 2 groups - domainSpecPattern = Pattern.compile(SPFTermsRegexps.DOMAIN_SPEC_REGEX_R); + domainSpecPattern = Pattern.compile(SPFTermsRegexps.DOMAIN_SPEC_REGEX); // The real pattern replacer macroStringPattern = Pattern.compile(SPFTermsRegexps.MACRO_STRING_REGEX_TOKEN); // The macro letters pattern @@ -221,23 +220,10 @@ public class MacroExpand { LOGGER.debug("Start expand domain: {}", input); - Matcher inputMatcher = domainSpecPattern.matcher(input); - if (!inputMatcher.matches() || inputMatcher.groupCount() != 2) { + if (!domainSpecPattern.matcher(input).matches()) { throw new PermErrorException("Invalid DomainSpec: "+input); } - StringBuffer res = new StringBuffer(); - if (inputMatcher.group(1) != null && inputMatcher.group(1).length() > 0) { - res.append(expandMacroString(inputMatcher.group(1), macroData, false)); - } - if (inputMatcher.group(2) != null && inputMatcher.group(2).length() > 0) { - if (inputMatcher.group(2).startsWith(".")) { - res.append(inputMatcher.group(2)); - } else { - res.append(expandMacroString(inputMatcher.group(2), macroData, false)); - } - } - String domainName = expandMacroString(input, macroData, false); if (domainName.indexOf('\r') >= 0 || domainName.indexOf('\n') >= 0) { throw new PermErrorException("Macro-expanded domain contains illegal CR or LF characters"); diff --git a/resolver/src/main/java/org/apache/james/jspf/core/SPFTermsRegexps.java b/resolver/src/main/java/org/apache/james/jspf/core/SPFTermsRegexps.java index a1afd0f..a0a9da4 100644 --- a/resolver/src/main/java/org/apache/james/jspf/core/SPFTermsRegexps.java +++ b/resolver/src/main/java/org/apache/james/jspf/core/SPFTermsRegexps.java @@ -49,11 +49,33 @@ public interface SPFTermsRegexps { + "|" + MACRO_LITERAL_REGEX + "{1}"; + /** + * The macro-expands of a macro-string, with the macro-literals before each of them. + * + * Possessive: java.util.regex recurses once per iteration of a repeated group it may backtrack + * into, which overflows the stack on a long macro-string, but loops over a possessive one. A + * macro-literal is never "%", so a macro-string splits into its tokens one way only: there is + * nothing to backtrack. + */ + final String MACRO_EXPANDS_REGEX = "(?:" + MACRO_LITERAL_REGEX + "*+" + MACRO_EXPAND_REGEX + ")*+"; + /** * ABNF: macro-string = *( macro-expand / macro-literal ) + * + * @deprecated not used by jSPF any more: it recurses once per character, which overflows the + * stack on a long macro-string. Use POSSESSIVE_MACRO_STRING_REGEX where nothing + * follows it, or DOMAIN_SPEC_REGEX. */ + @Deprecated final String MACRO_STRING_REGEX = "(?:" + MACRO_STRING_REGEX_TOKEN +")*"; + /** + * ABNF: macro-string = *( macro-expand / macro-literal ), matched possessively (see + * MACRO_EXPANDS_REGEX): it never gives a character back, so nothing that can be a macro-literal + * or a macro-expand may follow it, as at the end of a term. + */ + final String POSSESSIVE_MACRO_STRING_REGEX = MACRO_EXPANDS_REGEX + MACRO_LITERAL_REGEX + "*+"; + final String ALPHA_DIGIT_PATTERN = "[a-zA-Z0-9]"; /** @@ -64,8 +86,7 @@ public interface SPFTermsRegexps { final String TOP_LABEL_REGEX = "(?:" + ALPHA_DIGIT_PATTERN + "*" + SPFTermsRegexps.ALPHA_PATTERN + "{1}" + ALPHA_DIGIT_PATTERN + "*|(?:" - + ALPHA_DIGIT_PATTERN + "+" + "\\-" + "(?:" - + ALPHA_DIGIT_PATTERN + "|\\-)*" + + ALPHA_DIGIT_PATTERN + "+" + "\\-" + "[a-zA-Z0-9\\-]*" + ALPHA_DIGIT_PATTERN + "))"; /** @@ -76,13 +97,22 @@ public interface SPFTermsRegexps { /** * ABNF: domain-spec = macro-string domain-end + * + * Matched without backtracking into the macro-expands (see MACRO_EXPANDS_REGEX): either the + * macro-expands then macro-literals ending with "." toplabel [ "." ], or macro-literals and + * macro-expands ending with a macro-expand. So a pattern appended to it cannot take a + * macro-expand back: what follows must hold none, as the CIDR lengths the terms append. */ - final String DOMAIN_SPEC_REGEX = "(" - + SPFTermsRegexps.MACRO_STRING_REGEX + DOMAIN_END_REGEX + ")"; + final String DOMAIN_SPEC_REGEX = "((?:" + MACRO_EXPANDS_REGEX + MACRO_LITERAL_REGEX + "*\\." + + TOP_LABEL_REGEX + "\\.?" + "|(?:" + MACRO_LITERAL_REGEX + "*+" + MACRO_EXPAND_REGEX + ")++))"; /** * Spring MACRO_STRING from DOMAIN_END (domain end starts with .) + * + * @deprecated not used by jSPF any more: its macro-string group recurses once per character, + * which overflows the stack on a long domain-spec. Use DOMAIN_SPEC_REGEX. */ + @Deprecated final String DOMAIN_SPEC_REGEX_R = "(" + SPFTermsRegexps.MACRO_STRING_REGEX + ")(" + DOMAIN_END_REGEX + ")"; diff --git a/resolver/src/main/java/org/apache/james/jspf/terms/UnknownModifier.java b/resolver/src/main/java/org/apache/james/jspf/terms/UnknownModifier.java index 519bea1..a5c84c3 100644 --- a/resolver/src/main/java/org/apache/james/jspf/terms/UnknownModifier.java +++ b/resolver/src/main/java/org/apache/james/jspf/terms/UnknownModifier.java @@ -37,7 +37,7 @@ public class UnknownModifier implements Modifier, ConfigurationEnabled { */ public static final String REGEX = "(" + SPFTermsRegexps.ALPHA_PATTERN + "{1}" + "[A-Za-z0-9\\-\\_\\.]*" + ")" + "\\=(" - + SPFTermsRegexps.MACRO_STRING_REGEX + ")"; + + SPFTermsRegexps.POSSESSIVE_MACRO_STRING_REGEX + ")"; /** * @see org.apache.james.jspf.core.SPFChecker#checkSPF(org.apache.james.jspf.core.SPFSession) diff --git a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java index 26f9b80..66186b7 100644 --- a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java @@ -36,6 +36,14 @@ import static org.junit.Assert.assertEquals; public abstract class SPFExecutorBaseTest { protected final String zonesDir = "src/test/resources/dnszones"; + // one TXT record made of 255 byte strings, as DNS carries a long record + private static TXTRecord longTxtRecord(String hostname, String record) throws TextParseException { + List<String> strings = IntStream.range(0, (record.length() + 254) / 255) + .mapToObj(i -> record.substring(i * 255, Math.min(record.length(), (i + 1) * 255))) + .collect(Collectors.toList()); + return new TXTRecord(Name.fromString(hostname + "."), DClass.IN, 30L, strings); + } + protected abstract SPF createSPF(); protected abstract SPF createCustomSPF(DNSService dnsService); @@ -352,17 +360,27 @@ public abstract class SPFExecutorBaseTest { @Test(timeout = 10000) public void shouldHandleRecordWithManyTerms() throws IOException { String hostname = "shouldHandleRecordWithManyTerms.spfexecutor.fake"; - String record = "v=spf1 " + String.join(" ", Collections.nCopies(3000, "ip4:192.0.2.1")) + " -all"; - // one TXT record made of 255 byte strings, as DNS carries it - List<String> strings = IntStream.range(0, (record.length() + 254) / 255) - .mapToObj(i -> record.substring(i * 255, Math.min(record.length(), (i + 1) * 255))) - .collect(Collectors.toList()); FakeResolver fakeResolver = new FakeResolver(); - fakeResolver.addRecord(new TXTRecord(Name.fromString(hostname + "."), DClass.IN, 30L, strings)); + fakeResolver.addRecord(longTxtRecord(hostname, + "v=spf1 " + String.join(" ", Collections.nCopies(3000, "ip4:192.0.2.1")) + " -all")); SPF spf = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)); SPFResult result = spf.checkSPF("1.2.3.4", "a_user@" + hostname, hostname); assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); } + + @Test(timeout = 10000) + public void shouldHandleRecordWithLongDomainSpec() throws IOException { + String hostname = "shouldHandleRecordWithLongDomainSpec.spfexecutor.fake"; + FakeResolver fakeResolver = new FakeResolver(); + // the include target, truncated to 255 characters from the left, is a subdomain of hostname, + // with no record: FakeResolver answers it without querying the real DNS + fakeResolver.addRecord(longTxtRecord(hostname, "v=spf1 include:" + "%{d}.".repeat(5000) + hostname + " -all")); + + SPF spf = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)); + SPFResult result = spf.checkSPF("1.2.3.4", "a_user@" + hostname, hostname); + + assertEquals(SPFErrorConstants.PERM_ERROR_CONV, result.getResult()); + } } diff --git a/resolver/src/test/java/org/apache/james/jspf/core/MacroExpandTest.java b/resolver/src/test/java/org/apache/james/jspf/core/MacroExpandTest.java index 020ab69..abef56b 100644 --- a/resolver/src/test/java/org/apache/james/jspf/core/MacroExpandTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/core/MacroExpandTest.java @@ -145,6 +145,20 @@ public class MacroExpandTest extends TestCase { assertEquals("strong", defIp4me.expand("%{l1r-}", new rfcIP4MacroData(), MacroExpand.DOMAIN)); } + public void testLongDomainSpecIsTruncatedWithoutOverflowingTheStack() throws PermErrorException { + String domain = defIp4me.expand("%{d}.".repeat(5000) + "com", new rfcIP4MacroData(), MacroExpand.DOMAIN); + + assertTrue(domain.length() <= 255); + assertTrue(domain.endsWith(".email.example.com.com")); + } + + public void testLongLiteralDomainSpecIsTruncatedWithoutOverflowingTheStack() throws PermErrorException { + String domain = defIp4me.expand("a.".repeat(5000) + "com", new rfcIP4MacroData(), MacroExpand.DOMAIN); + + assertTrue(domain.length() <= 255); + assertTrue(domain.endsWith(".a.com")); + } + public void testExample1() throws PermErrorException { assertEquals("3.2.0.192.in-addr._spf.example.com", defIp4me .expand("%{ir}.%{v}._spf.%{d2}", new rfcIP4MacroData(), MacroExpand.DOMAIN)); diff --git a/resolver/src/test/java/org/apache/james/jspf/core/SPFTermsRegexpsTest.java b/resolver/src/test/java/org/apache/james/jspf/core/SPFTermsRegexpsTest.java new file mode 100644 index 0000000..cd8ff3f --- /dev/null +++ b/resolver/src/test/java/org/apache/james/jspf/core/SPFTermsRegexpsTest.java @@ -0,0 +1,50 @@ +/**************************************************************** + * 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.james.jspf.core; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.Test; + +public class SPFTermsRegexpsTest { + + @Test + @SuppressWarnings("deprecation") + public void macroStringRegexShouldLetAPatternAppendToIt() { + // as code outside jSPF may build a domain-spec from it + Pattern domainSpec = Pattern.compile(SPFTermsRegexps.MACRO_STRING_REGEX + SPFTermsRegexps.DOMAIN_END_REGEX); + + assertTrue(domainSpec.matcher("%{i}.example.com").matches()); + } + + @Test + @SuppressWarnings("deprecation") + public void domainSpecRegexRShouldSplitTheMacroStringFromTheDomainEnd() { + Matcher matcher = Pattern.compile(SPFTermsRegexps.DOMAIN_SPEC_REGEX_R).matcher("%{i}.example.com"); + + assertTrue(matcher.matches()); + assertEquals("%{i}.example", matcher.group(1)); + assertEquals(".com", matcher.group(2)); + } +} diff --git a/resolver/src/test/java/org/apache/james/jspf/terms/UnknownModifierTest.java b/resolver/src/test/java/org/apache/james/jspf/terms/UnknownModifierTest.java new file mode 100644 index 0000000..7194c18 --- /dev/null +++ b/resolver/src/test/java/org/apache/james/jspf/terms/UnknownModifierTest.java @@ -0,0 +1,40 @@ +/**************************************************************** + * 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.james.jspf.terms; + +import static org.junit.Assert.assertEquals; + +import org.apache.james.jspf.impl.DefaultTermsFactory; +import org.apache.james.jspf.parser.RFC4408SPF1Parser; +import org.junit.Test; + +public class UnknownModifierTest { + + @Test + public void shouldParseALongValue() throws Exception { + // its macro-string takes no stack frame per character + String directives = new RFC4408SPF1Parser(new DefaultTermsFactory()) + .parse("v=spf1 foo=" + "a".repeat(50000) + " -all") + .getDirectives() + .toString(); + + assertEquals("[-all]", directives); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
