This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 2a077961b5 Add validation of chunk extensions
2a077961b5 is described below
commit 2a077961b5d2170f745e7a26afe40989088de9ab
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Jan 27 19:33:00 2026 +0000
Add validation of chunk extensions
---
.../coyote/http11/filters/ChunkedInputFilter.java | 109 ++++++++-----
.../tomcat/util/http/parser/ChunkExtension.java | 126 +++++++++++++++
.../apache/tomcat/util/http/parser/HttpParser.java | 4 +
.../util/http/parser/LocalStrings.properties | 2 +
.../http11/filters/TestChunkedInputFilter.java | 91 +++++++++++
.../util/http/parser/TestChunkExtension.java | 176 +++++++++++++++++++++
webapps/docs/changelog.xml | 4 +
7 files changed, 474 insertions(+), 38 deletions(-)
diff --git a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
index de2457838b..62a7251ef9 100644
--- a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
+++ b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
@@ -30,6 +30,8 @@ import org.apache.coyote.http11.Constants;
import org.apache.coyote.http11.InputFilter;
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.buf.HexUtils;
+import org.apache.tomcat.util.http.parser.ChunkExtension;
+import org.apache.tomcat.util.http.parser.ChunkExtension.State;
import org.apache.tomcat.util.http.parser.HttpHeaderParser;
import org.apache.tomcat.util.http.parser.HttpHeaderParser.HeaderDataSource;
import org.apache.tomcat.util.http.parser.HttpHeaderParser.HeaderParseStatus;
@@ -106,7 +108,7 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
private volatile ParseState parseState = ParseState.CHUNK_HEADER;
private volatile boolean crFound = false;
private volatile int chunkSizeDigitsRead = 0;
- private volatile boolean parsingExtension = false;
+ private volatile State extensionState = null;
private final AtomicLong extensionSize = new AtomicLong(0);
private final HttpHeaderParser httpHeaderParser;
@@ -251,7 +253,7 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
parseState = ParseState.CHUNK_HEADER;
crFound = false;
chunkSizeDigitsRead = 0;
- parsingExtension = false;
+ extensionState = null;
extensionSize.set(0);
httpHeaderParser.recycle();
}
@@ -355,19 +357,38 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
}
byte chr = readChunk.get(readChunk.position());
- if (chr == Constants.CR || chr == Constants.LF) {
- parsingExtension = false;
+
+ if (extensionState != null) {
+ extensionState = ChunkExtension.parse(chr, extensionState);
+ if (extensionState == State.CR) {
+ if (!parseCRLF()) {
+ return false;
+ }
+ eol = true;
+ extensionState = null;
+ } else {
+ // Check the size
+ long extSize = extensionSize.incrementAndGet();
+ if (maxExtensionSize > -1 && extSize > maxExtensionSize) {
+
throwBadRequestException(sm.getString("chunkedInputFilter.maxExtension"));
+ }
+ }
+ } else if (chr == Constants.CR || chr == Constants.LF) {
if (!parseCRLF()) {
return false;
}
eol = true;
- } else if (chr == Constants.SEMI_COLON && !parsingExtension) {
- // First semicolon marks the start of the extension. Further
- // semicolons may appear to separate multiple chunk-extensions.
- // These need to be processed as part of parsing the
extensions.
- parsingExtension = true;
- extensionSize.incrementAndGet();
- } else if (!parsingExtension) {
+ } else if (chr == Constants.SEMI_COLON) {
+ /*
+ * First semicolon marks the start of the extension.
ChunkedExtension parser takes over for the
+ * remainder of the extension.
+ */
+ extensionState = State.PRE_NAME;
+ long extSize = extensionSize.incrementAndGet();
+ if (maxExtensionSize > -1 && extSize > maxExtensionSize) {
+ return false;
+ }
+ } else {
int charValue = HexUtils.getDec(chr);
if (charValue != -1 && chunkSizeDigitsRead < 8) {
chunkSizeDigitsRead++;
@@ -376,17 +397,9 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
// Isn't valid hex so this is an error condition
throwBadRequestException(sm.getString("chunkedInputFilter.invalidHeader"));
}
- } else {
- // Extension 'parsing'
- // Note that the chunk-extension is neither parsed nor
- // validated. Currently it is simply ignored.
- long extSize = extensionSize.incrementAndGet();
- if (maxExtensionSize > -1 && extSize > maxExtensionSize) {
-
throwBadRequestException(sm.getString("chunkedInputFilter.maxExtension"));
- }
}
- // Parsing the CRLF increments pos
+ // Parsing the CRLF increments position
if (!eol) {
readChunk.position(readChunk.position() + 1);
}
@@ -418,19 +431,47 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
}
byte chr = readChunk.get(readChunk.position());
- if (chr == Constants.CR || chr == Constants.LF) {
- parsingExtension = false;
+
+ if (extensionState != null) {
+ try {
+ extensionState = ChunkExtension.parse(chr, extensionState);
+ } catch (IOException ioe) {
+ /*
+ * Can't throw the exception here. Need to swallow it. It
will be thrown when parseChunkHeader()
+ * is called. Not very efficient but it is an error
condition for something that is hardly ever
+ * used.
+ */
+ return false;
+ }
+ if (extensionState == State.CR) {
+ if (!skipCRLF()) {
+ return false;
+ }
+ eol = true;
+ extensionState = null;
+ } else {
+ // Check the size
+ long extSize = extensionSize.incrementAndGet();
+ if (maxExtensionSize > -1 && extSize > maxExtensionSize) {
+ return false;
+ }
+ }
+ } else if (chr == Constants.CR || chr == Constants.LF) {
if (!skipCRLF()) {
return false;
}
eol = true;
- } else if (chr == Constants.SEMI_COLON && !parsingExtension) {
- // First semicolon marks the start of the extension. Further
- // semicolons may appear to separate multiple chunk-extensions.
- // These need to be processed as part of parsing the
extensions.
- parsingExtension = true;
- extensionSize.incrementAndGet();
- } else if (!parsingExtension) {
+ } else if (chr == Constants.SEMI_COLON) {
+ /*
+ * First semicolon marks the start of the extension.
ChunkedExtension parser takes over for the
+ * remainder of the extension.
+ */
+ extensionState = State.PRE_NAME;
+ long extSize = extensionSize.incrementAndGet();
+ if (maxExtensionSize > -1 && extSize > maxExtensionSize) {
+ return false;
+ }
+ } else {
int charValue = HexUtils.getDec(chr);
if (charValue != -1 && chunkSizeDigitsRead < 8) {
chunkSizeDigitsRead++;
@@ -439,17 +480,9 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
// Isn't valid hex so this is an error condition
return false;
}
- } else {
- // Extension 'parsing'
- // Note that the chunk-extension is neither parsed nor
- // validated. Currently it is simply ignored.
- long extSize = extensionSize.incrementAndGet();
- if (maxExtensionSize > -1 && extSize > maxExtensionSize) {
- return false;
- }
}
- // Parsing the CRLF increments pos
+ // Parsing the CRLF increments position
if (!eol) {
readChunk.position(readChunk.position() + 1);
}
diff --git a/java/org/apache/tomcat/util/http/parser/ChunkExtension.java
b/java/org/apache/tomcat/util/http/parser/ChunkExtension.java
new file mode 100644
index 0000000000..136956b56f
--- /dev/null
+++ b/java/org/apache/tomcat/util/http/parser/ChunkExtension.java
@@ -0,0 +1,126 @@
+/*
+ * 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.tomcat.util.http.parser;
+
+import java.io.IOException;
+
+import org.apache.tomcat.util.res.StringManager;
+
+/*
+ * Unlike other HTTP parsers, this is a stateless (state is held by the
calling code), streaming parser as chunk headers
+ * are read as part of the request body and it is not always possible to
buffer then entire chunk header in memory.
+ */
+public class ChunkExtension {
+
+ private static final StringManager sm =
StringManager.getManager(ChunkExtension.class);
+
+ public static State parse(byte b, State state) throws IOException {
+
+ char c = (char) (0xFF & b);
+
+ switch (state) {
+ case PRE_NAME:
+ if (HttpParser.isWhiteSpace(c)) {
+ return State.PRE_NAME;
+ } else if (HttpParser.isToken(c)) {
+ return State.NAME;
+ }
+ break;
+ case NAME:
+ if (HttpParser.isWhiteSpace(c)) {
+ return State.POST_NAME;
+ } else if (HttpParser.isToken(c)) {
+ return State.NAME;
+ } else if (c == '=') {
+ return State.EQUALS;
+ } else if (c == '\r') {
+ return State.CR;
+ }
+ break;
+ case POST_NAME:
+ if (HttpParser.isWhiteSpace(c)) {
+ return State.POST_NAME;
+ } else if (c == '=') {
+ return State.EQUALS;
+ } else if (c == '\r') {
+ return State.CR;
+ }
+ break;
+ case EQUALS:
+ if (HttpParser.isWhiteSpace(c)) {
+ return State.EQUALS;
+ } else if (HttpParser.isToken(c)) {
+ return State.VALUE;
+ } else if (c == '"') {
+ return State.QUOTED_VALUE;
+ }
+ break;
+ case VALUE:
+ if (HttpParser.isToken(c)) {
+ return State.VALUE;
+ } else if (HttpParser.isWhiteSpace(c)) {
+ return State.POST_VALUE;
+ } else if (c == ';') {
+ return State.PRE_NAME;
+ } else if (c == '\r') {
+ return State.CR;
+ }
+ break;
+ case QUOTED_VALUE:
+ if (c == '"') {
+ return State.POST_VALUE;
+ } else if (c == '\\' || c == 127) {
+ throw new
IOException(sm.getString("chunkExtension.invalid"));
+ } else if (c == '\t') {
+ return State.QUOTED_VALUE;
+ } else if (c > 31) {
+ return State.QUOTED_VALUE;
+ }
+ break;
+ case POST_VALUE:
+ if (HttpParser.isWhiteSpace(c)) {
+ return State.POST_VALUE;
+ } else if (c == ';') {
+ return State.PRE_NAME;
+ } else if (c == '\r') {
+ return State.CR;
+ }
+ break;
+ case CR:
+ break;
+ }
+
+ throw new IOException(sm.getString("chunkExtension.invalid"));
+ }
+
+
+ private ChunkExtension() {
+ // Tomcat doesn't use this data. It only parses it to ensure that it
is correctly formatted.
+ }
+
+
+ public enum State {
+ PRE_NAME,
+ NAME,
+ POST_NAME,
+ EQUALS,
+ VALUE,
+ QUOTED_VALUE,
+ POST_VALUE,
+ CR
+ }
+}
diff --git a/java/org/apache/tomcat/util/http/parser/HttpParser.java
b/java/org/apache/tomcat/util/http/parser/HttpParser.java
index 409237e7f0..8b41c52140 100644
--- a/java/org/apache/tomcat/util/http/parser/HttpParser.java
+++ b/java/org/apache/tomcat/util/http/parser/HttpParser.java
@@ -378,6 +378,10 @@ public class HttpParser {
}
+ public static boolean isWhiteSpace(int c) {
+ return c == 9 || c == 32;
+ }
+
public static boolean isAbsolutePath(int c) {
return DEFAULT.isAbsolutePathRelaxed(c);
}
diff --git a/java/org/apache/tomcat/util/http/parser/LocalStrings.properties
b/java/org/apache/tomcat/util/http/parser/LocalStrings.properties
index 07238a9424..36a1a91d7a 100644
--- a/java/org/apache/tomcat/util/http/parser/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/http/parser/LocalStrings.properties
@@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+chunkExtension.invalid=Invalid chunk extension data found
+
cookie.fallToDebug=\n\
\ Note: further occurrences of this error will be logged at DEBUG level.
cookie.invalidCookieValue=A cookie header was received [{0}] that contained an
invalid cookie. That cookie will be ignored.
diff --git a/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
b/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
index c38168d1a3..f1d5b3efb7 100644
--- a/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
+++ b/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
@@ -1022,4 +1022,95 @@ public class TestChunkedInputFilter extends
TomcatBaseTest {
*/
Assert.assertEquals("5,4", client.getResponseBody());
}
+
+
+ @Test
+ public void testExtension01() throws Exception {
+ doTestExtension("abc", true);
+ }
+
+
+ @Test
+ public void testExtension02() throws Exception {
+ doTestExtension("abc=def", true);
+ }
+
+
+ @Test
+ public void testExtension03() throws Exception {
+ doTestExtension(" a = b ", true);
+ }
+
+
+ @Test
+ public void testExtension04() throws Exception {
+ doTestExtension(" a = \"b\" ", true);
+ }
+
+
+ @Test
+ public void testExtension05() throws Exception {
+ doTestExtension("a=b=c", false);
+ }
+
+
+ @Test
+ public void testExtension06() throws Exception {
+ doTestExtension("a=b;", false);
+ }
+
+
+ @Test
+ public void testExtension07() throws Exception {
+ doTestExtension("a=\"aa\r\n\"", false);
+ }
+
+
+ private void doTestExtension(String extension, boolean ok) throws
Exception {
+ // Setup Tomcat instance
+ Tomcat tomcat = getTomcatInstance();
+
+ Assert.assertTrue(tomcat.getConnector().setProperty(
+ "maxExtensionSize", Integer.toString(EXT_SIZE_LIMIT)));
+
+ // No file system docBase required
+ Context ctx = getProgrammaticRootContext();
+
+ Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet(ok));
+ ctx.addServletMappingDecoded("/", "servlet");
+
+ tomcat.start();
+
+ // @formatter:off
+ String[] request = new String[] {
+ "POST /echo-params.jsp HTTP/1.1" + CRLF +
+ "Host: any" + CRLF +
+ "Transfer-encoding: chunked" + CRLF +
+
SimpleHttpClient.HTTP_HEADER_CONTENT_TYPE_FORM_URL_ENCODING +
+ "Connection: close" + CRLF +
+ CRLF +
+ "3;" + extension + CRLF +
+ "a=0" + CRLF +
+ "4" + CRLF +
+ "&b=1" + CRLF +
+ "0" + CRLF +
+ CRLF
+ };
+ // @formatter:on
+
+ TrailerClient client =
+ new TrailerClient(tomcat.getConnector().getLocalPort());
+ client.setRequest(request);
+
+ client.connect();
+ client.processRequest();
+
+ if (ok) {
+ Assert.assertTrue(client.isResponse200());
+ } else {
+ Assert.assertTrue(client.isResponse500());
+ }
+ }
+
+
}
diff --git a/test/org/apache/tomcat/util/http/parser/TestChunkExtension.java
b/test/org/apache/tomcat/util/http/parser/TestChunkExtension.java
new file mode 100644
index 0000000000..8bfd5bdfb9
--- /dev/null
+++ b/test/org/apache/tomcat/util/http/parser/TestChunkExtension.java
@@ -0,0 +1,176 @@
+/*
+ * 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.tomcat.util.http.parser;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.util.http.parser.ChunkExtension.State;
+
+public class TestChunkExtension {
+
+ @Test
+ public void testEmpty() {
+ doTest("\r\n", true);
+ }
+
+ @Test
+ public void testInvalid() {
+ doTest("x\r\n", false);
+ }
+
+ @Test
+ public void testNoToken01() {
+ doTest(";\r\n", false);
+ }
+
+ @Test
+ public void testNoToken02() {
+ doTest(" ;\r\n", false);
+ }
+
+ @Test
+ public void testNoToken03() {
+ doTest("; \r\n", false);
+ }
+
+ @Test
+ public void testNoToken04() {
+ doTest(";\t\r\n", false);
+ }
+
+ @Test
+ public void testInvalidToken01() {
+ doTest("; =\r\n", false);
+ }
+
+ @Test
+ public void testTokenOnly01() {
+ doTest("; abc\r\n", true);
+ }
+
+ @Test
+ public void testTokenOnly02() {
+ doTest("; abc \r\n", true);
+ }
+
+ @Test
+ public void testTokenOnly03() {
+ doTest("; abc \r\n", true);
+ }
+
+ @Test
+ public void testTokenToken01() {
+ doTest(";abc=abc\r\n", true);
+ }
+
+ @Test
+ public void testTokenToken02() {
+ doTest("; abc = abc \r\n", true);
+ }
+
+ @Test
+ public void testTokenQs01() {
+ doTest("; abc =\"\"\r\n", true);
+ }
+
+ @Test
+ public void testTokenQs02() {
+ doTest("; abc =\"abc\"\r\n", true);
+ }
+
+ @Test
+ public void testTokenQs03() {
+ doTest("; abc =\"a\tbc\"\r\n", true);
+ }
+
+ @Test
+ public void testTokenInvalidQs01() {
+ doTest("; abc =\"a\rbc\"\r\n", false);
+ }
+
+ @Test
+ public void testTokenInvalidQs02() {
+ doTest("; abc =\"a\\bc\"\r\n", false);
+ }
+
+ @Test
+ public void testTokenInvalidQs03() {
+ doTest("; abc =\"a\u007f\"\r\n", false);
+ }
+
+ @Test
+ public void testTokenInvalid01() {
+ doTest("; abc =\r\n", false);
+ }
+
+ @Test
+ public void testTokenInvalid02() {
+ doTest("; abc ==\r\n", false);
+ }
+
+ @Test
+ public void testTokenInvalid03() {
+ doTest(";a=b=c\r\n", false);
+ }
+
+ @Test
+ public void testTokenInvalid04() {
+ doTest(";a\"r\n", false);
+ }
+
+ @Test
+ public void testTokenInvalid05() {
+ doTest(";a \"r\n", false);
+ }
+
+ @Test
+ public void testValidValid() {
+ doTest(";abc=def;ghi=jkl\r\n", true);
+ }
+
+ @Test
+ public void testValidInvalid() {
+ doTest(";abc=def;=\r\n", false);
+ }
+
+ private void doTest(String input, boolean valid) {
+ byte[] bytes = input.getBytes(StandardCharsets.ISO_8859_1);
+
+ try {
+ // This state assumes either ';' or CRLF will follow, preceded by
optional white space.
+ State state = State.POST_VALUE;
+ for (byte b : bytes) {
+ state = ChunkExtension.parse(b, state);
+ /*
+ * The test values all end in \r\n but ChunkExtension only
looks for \r. In real usage the
+ * ChunkedInputFilter then parses the CRLF.
+ */
+ if (state == State.CR) {
+ break;
+ }
+ }
+ Assert.assertTrue("The input was invalid but no exception was
thrown", valid);
+ Assert.assertEquals("Parsing ended at state other than CR",
State.CR, state);
+ } catch (IOException ioe) {
+ Assert.assertFalse("The input was valid but an exception was
thrown", valid);
+ }
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index eae2c7bea2..f37bc3cb11 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -293,6 +293,10 @@
stream reset or a 400 response as appropriate rather then with a
connection reset. (markt)
</fix>
+ <fix>
+ Add validation of chunk extensions for chunked transfer encoding.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
