This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/master by this push:
new 5999e6a345 [FIX] Prevent stackoverflow in IMAP SEARCH (#3007)
5999e6a345 is described below
commit 5999e6a345ca6a67f215c032811fa735cd520152
Author: Benoit TELLIER <[email protected]>
AuthorDate: Mon Apr 13 07:16:11 2026 +0200
[FIX] Prevent stackoverflow in IMAP SEARCH (#3007)
---
.../imap/decode/parser/SearchCommandParser.java | 57 +++++----
.../SearchCommandParserNestingDepthTest.java | 132 +++++++++++++++++++++
2 files changed, 165 insertions(+), 24 deletions(-)
diff --git
a/protocols/imap/src/main/java/org/apache/james/imap/decode/parser/SearchCommandParser.java
b/protocols/imap/src/main/java/org/apache/james/imap/decode/parser/SearchCommandParser.java
index efb244940b..4f76e6a83e 100644
---
a/protocols/imap/src/main/java/org/apache/james/imap/decode/parser/SearchCommandParser.java
+++
b/protocols/imap/src/main/java/org/apache/james/imap/decode/parser/SearchCommandParser.java
@@ -67,6 +67,7 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
}
private static final Logger LOGGER =
LoggerFactory.getLogger(SearchCommandParser.class);
+ private static final int MAX_NESTING_DEPTH = 32;
@Inject
public SearchCommandParser(StatusResponseFactory statusResponseFactory) {
@@ -84,22 +85,26 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
* true when this is the first token read, false otherwise
*/
protected SearchKey searchKey(ImapSession session, ImapRequestLineReader
request, Context context, boolean isFirstToken) throws DecodingException,
IllegalCharsetNameException, UnsupportedCharsetException {
+ return searchKey(session, request, context, isFirstToken, 0);
+ }
+
+ private SearchKey searchKey(ImapSession session, ImapRequestLineReader
request, Context context, boolean isFirstToken, int depth) throws
DecodingException, IllegalCharsetNameException, UnsupportedCharsetException {
final char next = request.nextChar();
-
+
if (next >= '0' && next <= '9' || next == '*' || next == '$') {
return sequenceSet(session, request);
} else if (next == '(') {
- return paren(session, request, context);
+ return paren(session, request, context, depth + 1);
} else {
final int cap = consumeAndCap(request);
switch (cap) {
-
+
case 'A':
return a(request);
case 'B':
return b(request, context.getCharset());
case 'C':
- return c(session, request, isFirstToken, context);
+ return c(session, request, isFirstToken, context, depth);
case 'D':
return d(request);
case 'E':
@@ -121,9 +126,9 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
case 'M':
return modseq(request);
case 'N':
- return n(session, request, context);
+ return n(session, request, context, depth);
case 'O':
- return o(session, request, context);
+ return o(session, request, context, depth);
case 'P':
throw new
DecodingException(HumanReadableText.ILLEGAL_ARGUMENTS, "Unknown search key");
case 'Q':
@@ -164,21 +169,25 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
}
}
- private SearchKey paren(ImapSession session, ImapRequestLineReader
request, Context context) throws DecodingException {
+ private SearchKey paren(ImapSession session, ImapRequestLineReader
request, Context context, int depth) throws DecodingException {
+ if (depth > MAX_NESTING_DEPTH) {
+ throw new DecodingException(HumanReadableText.ILLEGAL_ARGUMENTS,
+ "Maximum SEARCH nesting depth of " + MAX_NESTING_DEPTH + "
exceeded");
+ }
request.consume();
List<SearchKey> keys = new ArrayList<>();
- addUntilParen(session, request, keys, context);
+ addUntilParen(session, request, keys, context, depth);
return SearchKey.buildAnd(keys);
}
- private void addUntilParen(ImapSession session, ImapRequestLineReader
request, List<SearchKey> keys, Context context) throws DecodingException {
+ private void addUntilParen(ImapSession session, ImapRequestLineReader
request, List<SearchKey> keys, Context context, int depth) throws
DecodingException {
final char next = request.nextWordChar();
if (next == ')') {
request.consume();
} else {
- final SearchKey key = searchKey(session, request, context, false);
+ final SearchKey key = searchKey(session, request, context, false,
depth);
keys.add(key);
- addUntilParen(session, request, keys, context);
+ addUntilParen(session, request, keys, context, depth);
}
}
@@ -195,19 +204,19 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
return result;
}
- private SearchKey c(ImapSession session, ImapRequestLineReader request,
boolean isFirstToken, Context context) throws DecodingException,
IllegalCharsetNameException, UnsupportedCharsetException {
+ private SearchKey c(ImapSession session, ImapRequestLineReader request,
boolean isFirstToken, Context context, int depth) throws DecodingException,
IllegalCharsetNameException, UnsupportedCharsetException {
final int next = consumeAndCap(request);
switch (next) {
case 'C':
return cc(request, context.getCharset());
case 'H':
- return charset(session, request, isFirstToken, context);
+ return charset(session, request, isFirstToken, context, depth);
default:
throw new DecodingException(HumanReadableText.ILLEGAL_ARGUMENTS,
"Unknown search key");
}
}
- private SearchKey charset(ImapSession session, ImapRequestLineReader
request, boolean isFirstToken, Context context) throws DecodingException,
IllegalCharsetNameException, UnsupportedCharsetException {
+ private SearchKey charset(ImapSession session, ImapRequestLineReader
request, boolean isFirstToken, Context context, int depth) throws
DecodingException, IllegalCharsetNameException, UnsupportedCharsetException {
final SearchKey result;
nextIsA(request);
nextIsR(request);
@@ -222,7 +231,7 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
final Charset charset = Charset.forName(value);
request.nextWordChar();
context.setCharset(charset);
- result = searchKey(session, request, context, false);
+ result = searchKey(session, request, context, false, depth);
return result;
}
@@ -356,7 +365,7 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
}
}
- private SearchKey o(ImapSession session, ImapRequestLineReader request,
Context context) throws DecodingException {
+ private SearchKey o(ImapSession session, ImapRequestLineReader request,
Context context, int depth) throws DecodingException {
final int next = consumeAndCap(request);
switch (next) {
case 'L':
@@ -364,7 +373,7 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
case 'N':
return on(request);
case 'R':
- return or(session, request, context);
+ return or(session, request, context, depth);
default:
throw new DecodingException(HumanReadableText.ILLEGAL_ARGUMENTS,
"Unknown search key");
}
@@ -381,13 +390,13 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
}
}
- private SearchKey n(ImapSession session, ImapRequestLineReader request,
Context context) throws DecodingException {
+ private SearchKey n(ImapSession session, ImapRequestLineReader request,
Context context, int depth) throws DecodingException {
final int next = consumeAndCap(request);
switch (next) {
case 'E':
return newOperator(request);
case 'O':
- return not(session, request, context);
+ return not(session, request, context, depth);
default:
throw new DecodingException(HumanReadableText.ILLEGAL_ARGUMENTS,
"Unknown search key");
}
@@ -583,21 +592,21 @@ public class SearchCommandParser extends
AbstractUidCommandParser {
return result;
}
- private SearchKey or(ImapSession session, ImapRequestLineReader request,
Context context) throws DecodingException {
+ private SearchKey or(ImapSession session, ImapRequestLineReader request,
Context context, int depth) throws DecodingException {
final SearchKey result;
nextIsSpace(request);
- final SearchKey firstKey = searchKey(session, request, context, false);
+ final SearchKey firstKey = searchKey(session, request, context, false,
depth);
nextIsSpace(request);
- final SearchKey secondKey = searchKey(session, request, context,
false);
+ final SearchKey secondKey = searchKey(session, request, context,
false, depth);
result = SearchKey.buildOr(firstKey, secondKey);
return result;
}
- private SearchKey not(ImapSession session, ImapRequestLineReader request,
Context context) throws DecodingException {
+ private SearchKey not(ImapSession session, ImapRequestLineReader request,
Context context, int depth) throws DecodingException {
final SearchKey result;
nextIsT(request);
nextIsSpace(request);
- final SearchKey nextKey = searchKey(session, request, context, false);
+ final SearchKey nextKey = searchKey(session, request, context, false,
depth);
result = SearchKey.buildNot(nextKey);
return result;
}
diff --git
a/protocols/imap/src/test/java/org/apache/james/imap/decode/parser/SearchCommandParserNestingDepthTest.java
b/protocols/imap/src/test/java/org/apache/james/imap/decode/parser/SearchCommandParserNestingDepthTest.java
new file mode 100644
index 0000000000..c7069403b9
--- /dev/null
+++
b/protocols/imap/src/test/java/org/apache/james/imap/decode/parser/SearchCommandParserNestingDepthTest.java
@@ -0,0 +1,132 @@
+/****************************************************************
+ * 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.imap.decode.parser;
+
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
+
+import org.apache.james.imap.api.message.response.StatusResponseFactory;
+import org.apache.james.imap.decode.DecodingException;
+import org.apache.james.imap.decode.ImapRequestStreamLineReader;
+import org.apache.james.imap.decode.parser.SearchCommandParser.Context;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class SearchCommandParserNestingDepthTest {
+
+ private static final int MAX_NESTING_DEPTH = 32;
+
+ private SearchCommandParser parser;
+
+ @BeforeEach
+ void setUp() {
+ parser = new SearchCommandParser(mock(StatusResponseFactory.class));
+ }
+
+ private ImapRequestStreamLineReader reader(String input) {
+ return new ImapRequestStreamLineReader(
+ new
ByteArrayInputStream(input.getBytes(StandardCharsets.US_ASCII)),
+ new ByteArrayOutputStream());
+ }
+
+ /** Builds "(((...(ALL)...)))\r\n" with exactly {@code depth} levels of
parentheses. */
+ private String nested(int depth) {
+ return nestedNoEol(depth) + "\r\n";
+ }
+
+ /** Same without trailing CRLF, for embedding inside compound commands
(OR, NOT). */
+ private String nestedNoEol(int depth) {
+ return "(".repeat(depth) + "ALL" + ")".repeat(depth);
+ }
+
+ @Test
+ void nestingAtMaxDepthShouldBeAccepted() {
+ assertThatCode(() ->
+ parser.searchKey(null, reader(nested(MAX_NESTING_DEPTH)), new
Context(), false))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
+ void nestingBelowMaxDepthShouldBeAccepted() {
+ assertThatCode(() ->
+ parser.searchKey(null, reader(nested(MAX_NESTING_DEPTH - 1)), new
Context(), false))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
+ void nestingOneAboveMaxDepthShouldBeRejected() {
+ assertThatThrownBy(() ->
+ parser.searchKey(null, reader(nested(MAX_NESTING_DEPTH + 1)), new
Context(), false))
+ .isInstanceOf(DecodingException.class)
+ .hasMessageContaining("nesting depth");
+ }
+
+ @Test
+ void deeplyNestedPayloadShouldBeRejectedWithoutStackOverflow() {
+ // 3998 levels fits in 8000 chars (the Netty line limit).
+ // Before the fix this caused StackOverflowError; after the fix it must
+ // throw a clean DecodingException well before the stack is exhausted.
+ String payload = nested(3998);
+ assertThatThrownBy(() ->
+ parser.searchKey(null, reader(payload), new Context(), false))
+ .isInstanceOf(DecodingException.class)
+ .isNotInstanceOf(StackOverflowError.class)
+ .hasMessageContaining("nesting depth");
+ }
+
+ @Test
+ void notWrappingNestingAtMaxDepthShouldBeAccepted() {
+ String payload = "NOT " + nestedNoEol(MAX_NESTING_DEPTH) + "\r\n";
+ assertThatCode(() ->
+ parser.searchKey(null, reader(payload), new Context(), false))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
+ void notWrappingNestingAboveMaxDepthShouldBeRejected() {
+ String payload = "NOT " + nestedNoEol(MAX_NESTING_DEPTH + 1) + "\r\n";
+ assertThatThrownBy(() ->
+ parser.searchKey(null, reader(payload), new Context(), false))
+ .isInstanceOf(DecodingException.class)
+ .hasMessageContaining("nesting depth");
+ }
+
+ @Test
+ void orWithBothArgsAtMaxDepthShouldBeAccepted() {
+ String payload = "OR " + nestedNoEol(MAX_NESTING_DEPTH) + " " +
nestedNoEol(MAX_NESTING_DEPTH) + "\r\n";
+ assertThatCode(() ->
+ parser.searchKey(null, reader(payload), new Context(), false))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
+ void orWithOneArgAboveMaxDepthShouldBeRejected() {
+ String payload = "OR " + nestedNoEol(MAX_NESTING_DEPTH + 1) + "
ALL\r\n";
+ assertThatThrownBy(() ->
+ parser.searchKey(null, reader(payload), new Context(), false))
+ .isInstanceOf(DecodingException.class)
+ .hasMessageContaining("nesting depth");
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
