This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko-http.git
The following commit(s) were added to refs/heads/main by this push:
new 46d2ffc40 fix: drop HTTP/2 header fields containing CR, LF or NUL
(#1258)
46d2ffc40 is described below
commit 46d2ffc4002cdb28f7ad16c4dadeeb417058c8a7
Author: PJ Fanning <[email protected]>
AuthorDate: Tue Sep 1 20:38:28 2026 +0100
fix: drop HTTP/2 header fields containing CR, LF or NUL (#1258)
Motivation:
Outgoing HTTP/2 header fields were HPACK-encoded without checking for
CR, LF or NUL in the name or value. The HTTP/1.1 renderer scans each
rendered header for CR/LF and drops it, but the HTTP/2 path had no
equivalent, so an attacker-influenced value (a RawHeader, CustomHeader
or a response trailer built from user data) was encoded verbatim. RFC
9113 8.2.1 forbids these characters in a field name or value. On a
native HTTP/2 leg HPACK is length-prefixed so this is not direct frame
splitting, but it violates the spec and enables HTTP/2 -> HTTP/1.1
downgrade smuggling when an intermediary re-serialises the message,
and it means the mitigation an application relies on under HTTP/1.1
silently disappears under HTTP/2.
Modification:
In `HeaderCompression`, the single point every outgoing header field
(regular headers, trailers and pseudo-headers all arrive here as
key/value pairs) passes through before HPACK encoding, drop any field
whose name or value contains CR, LF or NUL, logging at debug. Debug
rather than warning because the value can be attacker-influenced, so a
warning would be a log-flooding vector, and the HTTP/1.1 renderer drops
silently too.
Result:
CR/LF/NUL in an HTTP/2 header name or value can no longer reach the
wire; the offending field is dropped, matching the HTTP/1.1 renderer,
whether it rides in the header block or a response trailer.
Tests:
- sbt "http2-tests/testOnly
org.apache.pekko.http.impl.engine.http2.Http2ServerSpec" - pass (125 tests);
two new tests assert a CRLF-bearing response header and a CRLF-bearing response
trailer header are dropped while a valid sibling trailer survives. Verified
both fail with the fix stashed (the injected set-cookie reaches the decoded
headers).
- sbt http-core/mimaReportBinaryIssues - pass (internal impl.engine.http2
change, no public API).
References:
None - aligns HTTP/2 header rendering with the HTTP/1.1 CR/LF guard (RFC
9113 8.2.1)
---
.../engine/http2/hpack/HeaderCompression.scala | 14 +++++++++-
.../http/impl/engine/http2/Http2ServerSpec.scala | 30 ++++++++++++++++++++++
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala
index 60fec77d8..52254b9e9 100644
---
a/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala
+++
b/http-core/src/main/scala/org/apache/pekko/http/impl/engine/http2/hpack/HeaderCompression.scala
@@ -35,6 +35,10 @@ private[http2] object HeaderCompression extends
GraphStage[FlowShape[FrameEvent,
val shape = FlowShape(eventsIn, eventsOut)
+ /** RFC 9113 8.2.1: NUL, CR and LF are never valid in an HTTP/2 field name
or value. */
+ private[http2] def hasIllegalChar(s: String): Boolean =
+ s.indexOf('\r') >= 0 || s.indexOf('\n') >= 0 || s.indexOf('\u0000') >= 0
+
def createLogic(inheritedAttributes: Attributes): GraphStageLogic =
new GraphStageLogic(shape) with StageLogging with InHandler with
OutHandler { logic =>
setHandlers(eventsIn, eventsOut, this)
@@ -55,7 +59,15 @@ private[http2] object HeaderCompression extends
GraphStage[FlowShape[FrameEvent,
else {
kvs.foreach {
case (key, value: String) =>
- encoder.encodeHeader(os, key, value, false)
+ // RFC 9113 8.2.1: a field name or value carrying a NUL, CR or
LF is malformed and must not be sent.
+ // The value can be attacker-controlled (a RawHeader,
CustomHeader or a response trailer), so drop the
+ // offending field instead of encoding it, mirroring the
HTTP/1.1 renderer's CR/LF guard.
+ if (HeaderCompression.hasIllegalChar(key) ||
HeaderCompression.hasIllegalChar(value))
+ // debug, not warning: the value can be attacker-influenced
(reflected into a header), so a warning
+ // here would be a log-flooding vector; the HTTP/1.1
renderer drops such headers silently too
+ log.debug("Dropping HTTP/2 header [{}] because its name or
value contains a CR, LF or NUL", key)
+ else
+ encoder.encodeHeader(os, key, value, false)
case (key, value) =>
throw new IllegalStateException(
s"Didn't expect key-value-pair [$key] ->
[$value](${value.getClass}) here.")
diff --git
a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
index 8183220fa..78bd9d78b 100644
---
a/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
+++
b/http2-tests/src/test/scala/org/apache/pekko/http/impl/engine/http2/Http2ServerSpec.scala
@@ -316,6 +316,36 @@ class Http2ServerSpec extends Http2SpecWithMaterializer("""
trailingResponseHeaders.size should be(1)
trailingResponseHeaders.head should be(("Status", "grpc-status 10"))
})
+ "drop a response header whose value contains
CRLF".inAssertAllStagesStopped(
+ new TestSetup with RequestResponseProbes {
+ val streamId = 1
+ network.sendHEADERS(streamId, endStream = true,
network.headersForRequest(Get("/")))
+ user.expectRequest()
+ user.emitResponse(streamId,
+ HttpResponse(StatusCodes.OK, headers = RawHeader("x-trace",
"ok\r\nset-cookie: injected=1") :: Nil))
+
+ val responseHeaders =
network.expectDecodedResponseHEADERSPairs(streamId)
+ responseHeaders.map(_._1) should not contain "x-trace"
+ responseHeaders.exists { case (_, v) => v.contains("injected") }
should be(false)
+ })
+ "drop a response trailer header whose value contains
CRLF".inAssertAllStagesStopped(
+ new TestSetup with RequestResponseProbes {
+ val streamId = 1
+ network.sendHEADERS(streamId, endStream = true,
network.headersForRequest(Get("/")))
+ user.expectRequest()
+ val response =
+ HttpResponse(StatusCodes.OK,
+ entity =
HttpEntity.Strict(ContentTypes.`application/octet-stream`, ByteString("Hello")))
+ .addAttribute(AttributeKeys.trailer,
+ Trailer(Vector(RawHeader("x-evil", "ok\r\nset-cookie:
injected=1"), RawHeader("x-good", "fine"))))
+ user.emitResponse(streamId, response)
+
+ network.expectHeaderBlock(streamId, endStream = false)
+ network.expectDATA(streamId, endStream = false, ByteString("Hello"))
+ val trailingResponseHeaders =
network.expectDecodedResponseHEADERSPairs(streamId)
+ trailingResponseHeaders should contain(("x-good", "fine"))
+ trailingResponseHeaders.map(_._1) should not contain "x-evil"
+ })
"consider stream as closed after sending out strict response >
WINDOW_SIZE".inAssertAllStagesStopped(
new TestSetup with RequestResponseProbes {
override def settings: ServerSettings =
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
