fix: add header validation to early_hints callback#3588
Conversation
The early_hints callback constructs 103 Early Hints responses without any header validation, while process_headers validates against TOKEN_RE and HEADER_VALUE_RE for normal responses. This inconsistency means a WSGI app passing unsanitized data to wsgi.early_hints could enable HTTP response splitting via CRLF injection. Apply the same TOKEN_RE/HEADER_VALUE_RE checks from process_headers to the early_hints callback for defense-in-depth consistency. Closes benoitc#3585
eddieran
force-pushed
the
fix/early-hints-header-validation
branch
from
a58c122 to
7ae6503
Compare
| if not TOKEN_RE.fullmatch(name): | ||
| raise InvalidHeaderName('%r' % name) | ||
| if not HEADER_VALUE_RE.fullmatch(value): | ||
| raise InvalidHeader('%r' % value) |
There was a problem hiding this comment.
Until error handling receives some major cleanup (preferably class inheritance instead of massive elif) that exception class should be instantiated with just the name, not the value. Otherwise any way of triggering the exception on purpose can be used move sensitive information between security boundaries. Browsers are picky about what headers can be accessed cross-site, and proxies sitting between Gunicorn and the client might pick which ones to forward. Best not to interfere with that.
Per @pajod review: the invalid header value may carry sensitive content, and raising it through the exception could leak it across security boundaries (browsers/proxies handling response splitting errors). Pass just the name instead.
|
Addressed, @pajod — thanks for catching the cross-boundary leak. Changed |
3 participants
Summary
Fixes #3585 — the
wsgi.early_hintscallback constructs 103 Early Hints responses with zero header validation, whileResponse.process_headersalready validates header names againstTOKEN_REand header values againstHEADER_VALUE_RE. This gap means a WSGI app passing unsanitized user input towsgi.early_hintscould enable HTTP response splitting via CRLF injection.What this does
Adds the same
TOKEN_RE.fullmatch(name)/HEADER_VALUE_RE.fullmatch(value)checks (and thevalue.strip(" \t")normalization) fromprocess_headersintosend_early_hints. Raises the sameInvalidHeaderName/InvalidHeaderexceptions on failure.Addressing maintainer concerns
Agreed — frameworks should validate. But gunicorn already validates in
process_headersfor normal responses, so this PR simply bringsearly_hintsto parity. It's defense-in-depth: if the framework misses it, the server catches it. The regex checks are fast and only run on the (typically small) early hints header list, so the cost is negligible.Test plan
InvalidHeaderraised, nothing written to socketInvalidHeaderNameraisedInvalidHeaderNameraised