Skip to content

Commit edb55fd

Browse files
normanmaurerchrisvest
normanmaurer
and
chrisvest
authored
Merge commit from fork (#15611)
* Prevent HTTP request/response smuggling via chunk encoding Motivation: Transfer-Encoding: chunked has some strict rules around parsing CR LF delimiters. If we are too lenient, it can cause request/response smuggling issues when combined with proxies that are lenient in different ways. See https://w4ke.info/2025/06/18/funky-chunks.html for the details. Modification: - Make sure that we reject chunks with chunk-extensions that contain lone Line Feed octets without their preceding Carriage Return octet. - Make sure that we issue HttpContent objects with decoding failures, if we decode a chunk and it isn't immediately followed by a CR LF octet pair. Result: Smuggling requests/responses is no longer possible. Fixes #15522 * Enforce CR LF line separators for HTTP messages by default But also make it configurable through `HttpDecoderConfig`, and add a system property opt-out to change the default back. * Remove property for the name of the strict line parsing property * Remove HeaderParser.parse overload that only takes a buffer argument Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
1 parent 15fdd92 commit edb55fd

8 files changed

Lines changed: 398 additions & 59 deletions

File tree

‎codec-http/src/main/java/io/netty/handler/codec/http/HttpDecoderConfig.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public final class HttpDecoderConfig implements Cloneable {
3434
private int maxInitialLineLength = HttpObjectDecoder.DEFAULT_MAX_INITIAL_LINE_LENGTH;
3535
private int maxHeaderSize = HttpObjectDecoder.DEFAULT_MAX_HEADER_SIZE;
3636
private int initialBufferSize = HttpObjectDecoder.DEFAULT_INITIAL_BUFFER_SIZE;
37+
private boolean strictLineParsing = HttpObjectDecoder.DEFAULT_STRICT_LINE_PARSING;
3738

3839
public int getInitialBufferSize() {
3940
return initialBufferSize;
@@ -217,6 +218,35 @@ public HttpDecoderConfig setTrailersFactory(HttpHeadersFactory trailersFactory)
217218
return this;
218219
}
219220

221+
public boolean isStrictLineParsing() {
222+
return strictLineParsing;
223+
}
224+
225+
/**
226+
* The RFC 9112 specification for the HTTP protocol says that the initial start-line, and the following header
227+
* field-lines, must be separated by a Carriage Return (CR) and Line Feed (LF) octet pair, but also offers that
228+
* implementations "MAY" accept just a Line Feed octet as a separator.
229+
* <p>
230+
* Parsing leniencies can increase compatibility with a wider range of implementations, but can also cause
231+
* security vulnerabilities, when multiple systems disagree on the meaning of leniently parsed messages.
232+
* <p>
233+
* When <em>strict line parsing</em> is enabled ({@code true}), then Netty will enforce that start- and header
234+
* field-lines MUST be separated by a CR LF octet pair, and will produce messagas with failed
235+
* {@link io.netty.handler.codec.DecoderResult}s.
236+
* <p>
237+
* When <em>strict line parsing</em> is disabled ({@code false}), then Netty will accept lone LF octets as line
238+
* seperators for the start- and header field-lines.
239+
* <p>
240+
* See <a href="https://datatracker.ietf.org/doc/html/rfc9112#name-message-format">RFC 9112 Section 2.1</a>.
241+
* @param strictLineParsing Whether <em>strict line parsing</em> should be enabled ({@code true}),
242+
* or not ({@code false}).
243+
* @return This decoder config.
244+
*/
245+
public HttpDecoderConfig setStrictLineParsing(boolean strictLineParsing) {
246+
this.strictLineParsing = strictLineParsing;
247+
return this;
248+
}
249+
220250
@Override
221251
public HttpDecoderConfig clone() {
222252
try {

‎codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java‎

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import io.netty.util.AsciiString;
2727
import io.netty.util.ByteProcessor;
2828
import io.netty.util.internal.StringUtil;
29+
import io.netty.util.internal.SystemPropertyUtil;
2930

3031
import java.util.List;
3132
import java.util.concurrent.atomic.AtomicBoolean;
@@ -151,6 +152,23 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
151152
public static final boolean DEFAULT_VALIDATE_HEADERS = true;
152153
public static final int DEFAULT_INITIAL_BUFFER_SIZE = 128;
153154
public static final boolean DEFAULT_ALLOW_DUPLICATE_CONTENT_LENGTHS = false;
155+
public static final boolean DEFAULT_STRICT_LINE_PARSING =
156+
SystemPropertyUtil.getBoolean("io.netty.handler.codec.http.defaultStrictLineParsing", true);
157+
158+
private static final Runnable THROW_INVALID_CHUNK_EXTENSION = new Runnable() {
159+
@Override
160+
public void run() {
161+
throw new InvalidChunkExtensionException();
162+
}
163+
};
164+
165+
private static final Runnable THROW_INVALID_LINE_SEPARATOR = new Runnable() {
166+
@Override
167+
public void run() {
168+
throw new InvalidLineSeparatorException();
169+
}
170+
};
171+
154172
private final int maxChunkSize;
155173
private final boolean chunkedSupported;
156174
private final boolean allowPartialChunks;
@@ -163,6 +181,7 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
163181
protected final HttpHeadersFactory trailersFactory;
164182
private final boolean allowDuplicateContentLengths;
165183
private final ByteBuf parserScratchBuffer;
184+
private final Runnable defaultStrictCRLFCheck;
166185
private final HeaderParser headerParser;
167186
private final LineParser lineParser;
168187

@@ -315,6 +334,7 @@ protected HttpObjectDecoder(HttpDecoderConfig config) {
315334
checkNotNull(config, "config");
316335

317336
parserScratchBuffer = Unpooled.buffer(config.getInitialBufferSize());
337+
defaultStrictCRLFCheck = config.isStrictLineParsing() ? THROW_INVALID_LINE_SEPARATOR : null;
318338
lineParser = new LineParser(parserScratchBuffer, config.getMaxInitialLineLength());
319339
headerParser = new HeaderParser(parserScratchBuffer, config.getMaxHeaderSize());
320340
maxChunkSize = config.getMaxChunkSize();
@@ -344,7 +364,7 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List<Object> ou
344364
case SKIP_CONTROL_CHARS:
345365
// Fall-through
346366
case READ_INITIAL: try {
347-
ByteBuf line = lineParser.parse(buffer);
367+
ByteBuf line = lineParser.parse(buffer, defaultStrictCRLFCheck);
348368
if (line == null) {
349369
return;
350370
}
@@ -449,11 +469,11 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List<Object> ou
449469
return;
450470
}
451471
/*
452-
* everything else after this point takes care of reading chunked content. basically, read chunk size,
472+
* Everything else after this point takes care of reading chunked content. Basically, read chunk size,
453473
* read chunk, read and ignore the CRLF and repeat until 0
454474
*/
455475
case READ_CHUNK_SIZE: try {
456-
ByteBuf line = lineParser.parse(buffer);
476+
ByteBuf line = lineParser.parse(buffer, THROW_INVALID_CHUNK_EXTENSION);
457477
if (line == null) {
458478
return;
459479
}
@@ -491,16 +511,16 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List<Object> ou
491511
// fall-through
492512
}
493513
case READ_CHUNK_DELIMITER: {
494-
final int wIdx = buffer.writerIndex();
495-
int rIdx = buffer.readerIndex();
496-
while (wIdx > rIdx) {
497-
byte next = buffer.getByte(rIdx++);
498-
if (next == HttpConstants.LF) {
514+
if (buffer.readableBytes() >= 2) {
515+
int rIdx = buffer.readerIndex();
516+
if (buffer.getByte(rIdx) == HttpConstants.CR &&
517+
buffer.getByte(rIdx + 1) == HttpConstants.LF) {
518+
buffer.skipBytes(2);
499519
currentState = State.READ_CHUNK_SIZE;
500-
break;
520+
} else {
521+
out.add(invalidChunk(buffer, new InvalidChunkTerminationException()));
501522
}
502523
}
503-
buffer.readerIndex(rIdx);
504524
return;
505525
}
506526
case READ_CHUNK_FOOTER: try {
@@ -723,7 +743,7 @@ private State readHeaders(ByteBuf buffer) {
723743

724744
final HeaderParser headerParser = this.headerParser;
725745

726-
ByteBuf line = headerParser.parse(buffer);
746+
ByteBuf line = headerParser.parse(buffer, defaultStrictCRLFCheck);
727747
if (line == null) {
728748
return null;
729749
}
@@ -745,7 +765,7 @@ private State readHeaders(ByteBuf buffer) {
745765
splitHeader(lineContent, startLine, lineLength);
746766
}
747767

748-
line = headerParser.parse(buffer);
768+
line = headerParser.parse(buffer, defaultStrictCRLFCheck);
749769
if (line == null) {
750770
return null;
751771
}
@@ -835,7 +855,7 @@ protected void handleTransferEncodingChunkedWithContentLength(HttpMessage messag
835855

836856
private LastHttpContent readTrailingHeaders(ByteBuf buffer) {
837857
final HeaderParser headerParser = this.headerParser;
838-
ByteBuf line = headerParser.parse(buffer);
858+
ByteBuf line = headerParser.parse(buffer, defaultStrictCRLFCheck);
839859
if (line == null) {
840860
return null;
841861
}
@@ -878,7 +898,7 @@ private LastHttpContent readTrailingHeaders(ByteBuf buffer) {
878898
name = null;
879899
value = null;
880900
}
881-
line = headerParser.parse(buffer);
901+
line = headerParser.parse(buffer, defaultStrictCRLFCheck);
882902
if (line == null) {
883903
return null;
884904
}
@@ -1147,7 +1167,7 @@ private static class HeaderParser {
11471167
this.maxLength = maxLength;
11481168
}
11491169

1150-
public ByteBuf parse(ByteBuf buffer) {
1170+
public ByteBuf parse(ByteBuf buffer, Runnable strictCRLFCheck) {
11511171
final int readableBytes = buffer.readableBytes();
11521172
final int readerIndex = buffer.readerIndex();
11531173
final int maxBodySize = maxLength - size;
@@ -1174,6 +1194,9 @@ public ByteBuf parse(ByteBuf buffer) {
11741194
// Drop CR if we had a CRLF pair
11751195
endOfSeqIncluded = indexOfLf - 1;
11761196
} else {
1197+
if (strictCRLFCheck != null) {
1198+
strictCRLFCheck.run();
1199+
}
11771200
endOfSeqIncluded = indexOfLf;
11781201
}
11791202
final int newSize = endOfSeqIncluded - readerIndex;
@@ -1209,18 +1232,18 @@ private final class LineParser extends HeaderParser {
12091232
}
12101233

12111234
@Override
1212-
public ByteBuf parse(ByteBuf buffer) {
1235+
public ByteBuf parse(ByteBuf buffer, Runnable strictCRLFCheck) {
12131236
// Suppress a warning because HeaderParser.reset() is supposed to be called
12141237
reset();
12151238
final int readableBytes = buffer.readableBytes();
12161239
if (readableBytes == 0) {
12171240
return null;
12181241
}
1219-
final int readerIndex = buffer.readerIndex();
1220-
if (currentState == State.SKIP_CONTROL_CHARS && skipControlChars(buffer, readableBytes, readerIndex)) {
1242+
if (currentState == State.SKIP_CONTROL_CHARS &&
1243+
skipControlChars(buffer, readableBytes, buffer.readerIndex())) {
12211244
return null;
12221245
}
1223-
return super.parse(buffer);
1246+
return super.parse(buffer, strictCRLFCheck);
12241247
}
12251248

12261249
private boolean skipControlChars(ByteBuf buffer, int readableBytes, int readerIndex) {

‎codec-http/src/main/java/io/netty/handler/codec/http/InvalidChunkExtensionException.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Copyright 2025 The Netty Project
3+
*
4+
* The Netty Project licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
package io.netty.handler.codec.http;
17+
18+
import io.netty.handler.codec.CorruptedFrameException;
19+
20+
/**
21+
* Thrown when HTTP chunk extensions could not be parsed, typically due to incorrect use of CR LF delimiters.
22+
* <p>
23+
* <a href="https://datatracker.ietf.org/doc/html/rfc9112#name-chunked-transfer-coding">RFC 9112</a>
24+
* specifies that chunk header lines must be terminated in a CR LF pair,
25+
* and that a lone LF octet is not allowed within the chunk header line.
26+
*/
27+
public final class InvalidChunkExtensionException extends CorruptedFrameException {
28+
private static final long serialVersionUID = 536224937231200736L;
29+
30+
public InvalidChunkExtensionException() {
31+
super("Line Feed must be preceded by Carriage Return when terminating HTTP chunk header lines");
32+
}
33+
34+
public InvalidChunkExtensionException(String message, Throwable cause) {
35+
super(message, cause);
36+
}
37+
38+
public InvalidChunkExtensionException(String message) {
39+
super(message);
40+
}
41+
42+
public InvalidChunkExtensionException(Throwable cause) {
43+
super(cause);
44+
}
45+
}

‎codec-http/src/main/java/io/netty/handler/codec/http/InvalidChunkTerminationException.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Copyright 2025 The Netty Project
3+
*
4+
* The Netty Project licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
package io.netty.handler.codec.http;
17+
18+
import io.netty.handler.codec.CorruptedFrameException;
19+
20+
/**
21+
* Thrown when HTTP chunks could not be parsed, typically due to incorrect use of CR LF delimiters.
22+
* <p>
23+
* <a href="https://datatracker.ietf.org/doc/html/rfc9112#name-chunked-transfer-coding">RFC 9112</a>
24+
* specifies that chunk bodies must be terminated in a CR LF pair,
25+
* and that the delimiter must follow the given chunk-size number of octets in chunk-data.
26+
*/
27+
public final class InvalidChunkTerminationException extends CorruptedFrameException {
28+
private static final long serialVersionUID = 536224937231200736L;
29+
30+
public InvalidChunkTerminationException() {
31+
super("Chunk data sections must be terminated by a CR LF octet pair");
32+
}
33+
34+
public InvalidChunkTerminationException(String message, Throwable cause) {
35+
super(message, cause);
36+
}
37+
38+
public InvalidChunkTerminationException(String message) {
39+
super(message);
40+
}
41+
42+
public InvalidChunkTerminationException(Throwable cause) {
43+
super(cause);
44+
}
45+
}

‎codec-http/src/main/java/io/netty/handler/codec/http/InvalidLineSeparatorException.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Copyright 2025 The Netty Project
3+
*
4+
* The Netty Project licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
package io.netty.handler.codec.http;
17+
18+
import io.netty.handler.codec.DecoderException;
19+
20+
/**
21+
* Thrown when {@linkplain HttpDecoderConfig#isStrictLineParsing() strict line parsing} is enabled,
22+
* and HTTP start- and header field-lines are not seperated by CR LF octet pairs.
23+
* <p>
24+
* Strict line parsing is enabled by default since Netty 4.1.124 and 4.2.4.
25+
* This default can be overridden by setting the {@value HttpObjectDecoder#PROP_DEFAULT_STRICT_LINE_PARSING} system
26+
* property to {@code false}.
27+
* <p>
28+
* See <a href="https://datatracker.ietf.org/doc/html/rfc9112#name-message-format">RFC 9112 Section 2.1</a>.
29+
*/
30+
public final class InvalidLineSeparatorException extends DecoderException {
31+
private static final long serialVersionUID = 536224937231200736L;
32+
33+
public InvalidLineSeparatorException() {
34+
super("Line Feed must be preceded by Carriage Return when terminating HTTP start- and header field-lines");
35+
}
36+
37+
public InvalidLineSeparatorException(String message, Throwable cause) {
38+
super(message, cause);
39+
}
40+
41+
public InvalidLineSeparatorException(String message) {
42+
super(message);
43+
}
44+
45+
public InvalidLineSeparatorException(Throwable cause) {
46+
super(cause);
47+
}
48+
}

0 commit comments

Comments
 (0)