diff --git a/modules/http/src/main/java/org/glassfish/grizzly/http/ChunkedTransferEncoding.java b/modules/http/src/main/java/org/glassfish/grizzly/http/ChunkedTransferEncoding.java index 6124506ff2..b605e0363c 100644 --- a/modules/http/src/main/java/org/glassfish/grizzly/http/ChunkedTransferEncoding.java +++ b/modules/http/src/main/java/org/glassfish/grizzly/http/ChunkedTransferEncoding.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2020 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2024 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -49,6 +49,9 @@ public final class ChunkedTransferEncoding implements TransferEncoding { private static final byte[] LAST_CHUNK_CRLF_BYTES = "0\r\n".getBytes(ASCII_CHARSET); private static final int[] DEC = HexUtils.getDecBytes(); + public static final String STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112 = "org.glassfish.grizzly.http.STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112"; + private static final boolean isStrictChunkedTransferCodingLineTerminatorSet = Boolean.parseBoolean(System.getProperty(STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112)); + private final int maxHeadersSize; public ChunkedTransferEncoding(final int maxHeadersSize) { @@ -247,6 +250,12 @@ private static boolean parseHttpChunkLength(final HttpPacketParsing httpPacket, b == Constants.CR || b == Constants.SEMI_COLON) { parsingState.checkpoint = offset; } else if (b == Constants.LF) { + if (isStrictChunkedTransferCodingLineTerminatorSet) { + if (parsingState.checkpoint2 == -1 || // no CR + parsingState.checkpoint2 != parsingState.checkpoint) { // not the previous CR or a repetition of a CR + throw new HttpBrokenContentException("Unexpected HTTP chunk header"); + } + } final ContentParsingState contentParsingState = httpPacket.getContentParsingState(); contentParsingState.chunkContentStart = offset + 1; contentParsingState.chunkLength = value; @@ -264,6 +273,11 @@ private static boolean parseHttpChunkLength(final HttpPacketParsing httpPacket, } else { throw new HttpBrokenContentException("Unexpected HTTP chunk header"); } + if (isStrictChunkedTransferCodingLineTerminatorSet) { + if (b == Constants.CR && parsingState.checkpoint2 == -1) { // first CR + parsingState.checkpoint2 = offset; + } + } offset++; } diff --git a/modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java b/modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java index 844e44e1bd..10ad25b003 100644 --- a/modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java +++ b/modules/http/src/test/java/org/glassfish/grizzly/http/ChunkedTransferEncodingTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2020 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2024 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -20,6 +20,7 @@ import static java.lang.Boolean.TRUE; import static java.util.Arrays.asList; import static java.util.concurrent.TimeUnit.SECONDS; +import static org.glassfish.grizzly.http.ChunkedTransferEncoding.STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112; import static org.glassfish.grizzly.http.HttpCodecFilter.DEFAULT_MAX_HTTP_PACKET_HEADER_SIZE; import static org.glassfish.grizzly.http.util.MimeHeaders.MAX_NUM_HEADERS_UNBOUNDED; import static org.glassfish.grizzly.memory.Buffers.EMPTY_BUFFER; @@ -90,6 +91,7 @@ static int PORT() { private final String eol; private final boolean isChunkWhenParsing; + private final boolean isStrictChunkedTransferCodingLineTerminatorSet; private TCPNIOTransport transport; private Connection connection; @@ -153,6 +155,8 @@ public void after() throws Exception { public ChunkedTransferEncodingTest(String eol, boolean isChunkWhenParsing) { this.eol = eol; this.isChunkWhenParsing = isChunkWhenParsing; + this.isStrictChunkedTransferCodingLineTerminatorSet = + Boolean.parseBoolean(System.getProperty(STRICT_CHUNKED_TRANSFER_CODING_LINE_TERMINATOR_RFC_9112)); } @Test @@ -258,8 +262,8 @@ public void testSpacesInChunkSizeHeader() throws Exception { sb.append("POST / HTTP/1.1\r\n"); sb.append("Host: localhost:").append(PORT).append("\r\n"); sb.append("Transfer-Encoding: chunked\r\n\r\n"); - sb.append(" ").append(msgLen).append(" ").append(eol).append(msg).append(eol); - sb.append(" 0 ").append(eol).append(eol); + sb.append(" ").append(msgLen).append(" ").append("\r\n").append(msg).append(eol); + sb.append(" 0 ").append("\r\n").append(eol); Buffer b = Buffers.wrap(DEFAULT_MEMORY_MANAGER, sb.toString(), Charsets.ASCII_CHARSET); Future f = connection.write(b); @@ -268,6 +272,65 @@ public void testSpacesInChunkSizeHeader() throws Exception { assertTrue(result.get(10, SECONDS)); } + @SuppressWarnings("unchecked") + @Test + public void testVulnerableLineTerminatorInChunkSizeHeader() throws Exception { + StringBuilder sb = new StringBuilder(); + String nestedMsg = "XX"; + String nestedMsgLen = Integer.toHexString(nestedMsg.length()); + sb.append("\r\n"); + sb.append("POST /2 HTTP/1.1").append("\r\n"); + sb.append("Host: localhost:").append(PORT).append("\r\n"); + sb.append("Transfer-Encoding: chunked").append("\r\n"); + sb.append("\r\n"); + sb.append(nestedMsgLen).append("\r\n"); + sb.append(nestedMsg).append("\r\n"); + String dummy = sb.toString(); + String firstMsg = "A".repeat(dummy.length()); + final String firstMsgLen = Integer.toHexString(firstMsg.length()); + + // original packet + sb = new StringBuilder(); + sb.append("POST /1 HTTP/1.1").append("\r\n"); + sb.append("Host: localhost:").append(PORT).append("\r\n"); + sb.append("Transfer-Encoding: chunked").append("\r\n"); + sb.append("\r\n"); + sb.append(firstMsgLen).append(';').append('\n').append(firstMsg).append('\n').append('0').append("\r\n"); + sb.append(dummy); + sb.append("0").append("\r\n"); // last-chunk + sb.append("\r\n"); // CRLF + + final Buffer expectedContent = Buffers.wrap(DEFAULT_MEMORY_MANAGER, firstMsg, ASCII_CHARSET); + httpRequestCheckFilter.setCheckParameters(expectedContent, Collections.>emptyMap()); + Buffer b = Buffers.wrap(DEFAULT_MEMORY_MANAGER, sb.toString(), Charsets.ASCII_CHARSET); + Future f = connection.write(b); + f.get(5, SECONDS); + + Future result; + if (!isStrictChunkedTransferCodingLineTerminatorSet) { + // first msg + result = resultQueue.poll(5, SECONDS); + assertTrue(result.get(2, SECONDS)); + + // nested msg + result = resultQueue.poll(5, SECONDS); + try { + result.get(2, SECONDS); + fail("Expected AssertError to be thrown on server side"); + } catch (ExecutionException ignore) { + } + } else { + // first msg + result = resultQueue.poll(5, SECONDS); + try { + result.get(2, SECONDS); + fail("Expected HttpBrokenContentException to be thrown on server side"); + } catch (ExecutionException ee) { + assertEquals(HttpBrokenContentException.class, ee.getCause().getClass()); + } + } + } + /** * Test private method {@link ChunkedTransferEncoding#checkOverflow(long)} via reflection. * @@ -319,10 +382,10 @@ private void doHttpRequestTest(int packetsNum, boolean hasContent, Map> entry : trailerHeaders.entrySet()) { final String value = entry.getValue().getFirst();