This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/pekko-http.git
The following commit(s) were added to refs/heads/1.4.x by this push:
new 5d840eb25 fix: reject whitespace between chunk-size digits (#1257)
(#1290)
5d840eb25 is described below
commit 5d840eb250c92c8716a75fedbcb01658053a2b97
Author: PJ Fanning <[email protected]>
AuthorDate: Thu Sep 10 10:55:19 2026 +0100
fix: reject whitespace between chunk-size digits (#1257) (#1290)
Motivation:
The chunk-size parser tolerates whitespace after the size digits
(illegal per the spec but seen in the wild, see #1812), but the `WSP`
arm recursed with the same accumulated size and then let parsing
continue with the next character — including a further hex digit. So a
chunk size such as `5 0` was read as `0x50` = 80 bytes, while a proxy
that stops at the space reads 5. That discrepancy is a request-
smuggling primitive when Pekko HTTP sits behind such a proxy. The
comment referencing #1812 intended only trailing whitespace; the
implementation skipped interior whitespace too. The `WSP` arm also had
no `cursor > offset` guard, so a size field of only spaces parsed as 0.
Modification:
Thread a `sawWhitespace` flag through `parseSize`. Once whitespace has
been seen, a hex digit no longer matches the accumulation arm and falls
through to the illegal-character case, so only more whitespace or a
terminator may follow. Guard the `WSP` arm with `cursor > offset` so
leading whitespace (before any digit) is rejected as well.
Result:
Trailing whitespace after the chunk size is still tolerated, but
whitespace between size digits, and leading whitespace, are rejected
with "Illegal character '...' in chunk start" — Pekko HTTP and a
fronting proxy can no longer disagree on the chunk size.
Tests:
- sbt "http-core/testOnly
org.apache.pekko.http.impl.engine.parsing.RequestParserCRLFSpec
org.apache.pekko.http.impl.engine.parsing.RequestParserLFSpec" - pass (116
tests); two new tests reject `5 0` (interior whitespace) and ` 5` (leading
whitespace), and the existing "incorrect but harmless whitespace after chunk
size" test still passes. Verified the interior-whitespace test fails with the
fix stashed (`5 0` parses as 80 bytes).
- sbt http-core/mimaReportBinaryIssues - pass (internal impl.engine.parsing
change, no public API).
References:
Refs #1812 - narrows the whitespace tolerance to trailing whitespace only
---
.../impl/engine/parsing/HttpMessageParser.scala | 18 ++++++++++++------
.../impl/engine/parsing/RequestParserSpec.scala | 22 ++++++++++++++++++++++
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala
index 0bfd35dfd..7d44ee613 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/HttpMessageParser.scala
@@ -311,23 +311,29 @@ private[http] trait HttpMessageParser[Output >:
MessageOutput <: ParserOutput] {
} else failEntityStream(
s"HTTP chunk extension length exceeds configured limit of
${settings.maxChunkExtLength} characters")
- @tailrec def parseSize(cursor: Int, size: Long): StateResult =
+ // `sawWhitespace` records that the size digits are over and we are in
trailing whitespace. Whitespace after the
+ // size is tolerated (illegal per the spec but seen in the wild, see issue
#1812), but whitespace *between* size
+ // digits is not: without this a chunk size such as `5 0` would be read
here as 0x50 = 80 bytes while a proxy that
+ // stops at the space reads 5, a request-smuggling discrepancy. Once
whitespace is seen only more whitespace or a
+ // terminator may follow; a further hex digit falls through to the
illegal-character case.
+ @tailrec def parseSize(cursor: Int, size: Long, sawWhitespace: Boolean):
StateResult =
if (size <= Int.MaxValue) {
byteChar(input, cursor) match {
- case c if CharacterClasses.HEXDIG(c) => parseSize(cursor + 1, size *
16 + CharUtils.hexValue(c))
+ case c if CharacterClasses.HEXDIG(c) && !sawWhitespace =>
+ parseSize(cursor + 1, size * 16 + CharUtils.hexValue(c),
sawWhitespace = false)
case c if size > settings.maxChunkSize =>
failEntityStream(
s"HTTP chunk of $size bytes exceeds the configured limit of
${settings.maxChunkSize} bytes")
case ';' if cursor > offset => parseChunkExtensions(size.toInt,
cursor + 1)()
case '\r' if cursor > offset && byteAt(input, cursor + 1) == LF_BYTE
=>
parseChunkBody(size.toInt, "", cursor + 2)
- case '\n' if cursor > offset => parseChunkBody(size.toInt, "",
cursor + 1)
- case c if CharacterClasses.WSP(c) => parseSize(cursor + 1, size) //
illegal according to the spec but can happen, see issue #1812
- case c => failEntityStream(s"Illegal
character '${escape(c)}' in chunk start")
+ case '\n' if cursor > offset =>
parseChunkBody(size.toInt, "", cursor + 1)
+ case c if CharacterClasses.WSP(c) && cursor > offset =>
parseSize(cursor + 1, size, sawWhitespace = true)
+ case c =>
failEntityStream(s"Illegal character '${escape(c)}' in chunk start")
}
} else failEntityStream(s"HTTP chunk size exceeds Integer.MAX_VALUE
(${Int.MaxValue}) bytes")
- try parseSize(offset, 0)
+ try parseSize(offset, 0, sawWhitespace = false)
catch {
case NotEnoughDataException => continue(input, offset)(parseChunk(_, _,
isLastMessage, totalBytesRead))
}
diff --git
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala
index fd9a160e3..38260d22d 100644
---
a/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala
+++
b/http-core/src/test/scala/org/apache/pekko/http/impl/engine/parsing/RequestParserSpec.scala
@@ -518,6 +518,28 @@ abstract class RequestParserSpec(mode: String, newLine:
String) extends AnyFreeS
closeAfterResponseCompletion shouldEqual Seq(false)
}
+ "whitespace between chunk size digits" in new Test {
+ // `5 0` must not be read as 0x50 = 80 bytes; a hex digit after
whitespace is rejected so that this parser and
+ // a fronting proxy cannot disagree on the chunk size (request
smuggling)
+ Seq(
+ start,
+ """5 0
+ |""") should generalMultiParseTo(
+ Right(baseRequest),
+ Left(EntityStreamError(ErrorInfo("Illegal character '0' in chunk
start"))))
+ closeAfterResponseCompletion shouldEqual Seq(false)
+ }
+
+ "leading whitespace before the chunk size" in new Test {
+ Seq(
+ start,
+ """ 5
+ |""") should generalMultiParseTo(
+ Right(baseRequest),
+ Left(EntityStreamError(ErrorInfo("Illegal character ' ' in chunk
start"))))
+ closeAfterResponseCompletion shouldEqual Seq(false)
+ }
+
"an illegal char in chunk size" in new Test {
Seq(start, "bla") should generalMultiParseTo(
Right(baseRequest),
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
