pjfanning opened a new pull request, #1256:
URL: https://github.com/apache/pekko-http/pull/1256
### Motivation
`RenderSupport.renderChunk` emitted two attacker-influenceable parts of a
chunked response **without** the CR/LF guard that the main header block relies
on. The guard lives in the `~~(header: HttpHeader)` overload
(`Rendering.scala`), which marks the position, renders the header, and then
scans the rendered bytes for CR/LF — dropping the header if any are found. Only
the main header loop went through it.
**Trailer headers** were rendered with `r ~~ trailer`, which resolves to the
generic sequence renderer (`Renderer[immutable.Iterable[HttpHeader]]`) and
calls `header.render(r)` directly, bypassing the guard:
```scala
case HttpEntity.LastChunk(_, trailer) => r ~~ trailer ~~ CrLf
```
`HttpEntity.LastChunk` and `RawHeader` perform no CR/LF validation, so a
trailer built from user data splits the response:
```scala
HttpEntity.Chunked(ct, Source(List(
HttpEntity.Chunk("body"),
HttpEntity.LastChunk(trailer = List(RawHeader("X-Trace",
"ok\r\nSet-Cookie: session=attacker"))))))
```
The identical `RawHeader` placed in the normal header list **is** caught by
the guard; only because it rides in the trailer did it reach the wire.
**The chunk extension** was rendered raw into the chunk-size line with no
CR/LF check:
```scala
if (extension.nonEmpty) r ~~ ';' ~~ extension
```
so a CR/LF in an app-set `HttpEntity.Chunk(data, extension)` corrupted the
chunk framing.
### Modification
- Render each trailer header through the guarded `~~(HttpHeader)` overload
(`trailer.foreach(r ~~ _)`), matching how the main header block renders
(`HttpResponseRendererFactory.render(h) = r ~~ h`).
- Remove the now-unused `trailerRenderer` implicit, so the unguarded
`Renderer[Iterable[HttpHeader]]` cannot be picked up again by accident.
- Skip a chunk extension containing CR/LF; the extension is optional
metadata, so omitting an illegal one is safe.
Byte output is unchanged for valid trailers and extensions (verified by the
existing rendering tests, which are exact-string comparisons).
### Result
CR/LF in a chunk trailer header value or a chunk extension can no longer
reach the wire — the offending header/extension is dropped, exactly as in the
main header block, instead of splitting the response.
### Tests
- `sbt "http-core/testOnly
org.apache.pekko.http.impl.engine.rendering.ResponseRendererSpec
org.apache.pekko.http.impl.engine.rendering.RequestRendererSpec"` — pass (62
tests). Two new tests assert that a CRLF-bearing trailer header and a
CRLF-bearing chunk extension are dropped from the rendered output. Verified
both fail with the fix stashed (the injected `Set-Cookie` bytes reach the
output).
- `sbt http-core/mimaReportBinaryIssues` — pass (internal
`impl.engine.rendering` change, no public API).
- Native `scalafmt` on the changed files — clean.
### References
None - closes the CRLF-injection paths in chunked response rendering
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
