Reject obs-fold and whitespace-before-colon in request headers (RFC 7230 3.2.4)#2661
Merged
Merged
Conversation
…230 3.2.4)
A header line using obsolete line folding (obs-fold) or with whitespace between
the field-name and the colon was parsed leniently and stored under a mangled
key. That let an obfuscated Transfer-Encoding ("Transfer-Encoding : chunked", a
tab before the colon, or a value carried on a folded continuation line) slip
past the header lookups that frame the request body: the server saw no
Transfer-Encoding/Content-Length, treated the body as empty, and left the
encoded payload in the keep-alive buffer where it was reparsed as a second,
smuggled request once a folding/whitespace-lenient front-end framed the body as
chunked (TE/CL desync, request smuggling).
try_parse_request now rejects both forms with 400 Bad Request and closes the
connection, as RFC 7230 3.2.4 requires; closing discards the leftover bytes.
Well-formed requests, including header values that contain spaces and colons,
are unaffected.
Complements #2654 (which rejects a clean Transfer-Encoding header); together
they close the obfuscated and non-obfuscated TE/CL desync variants.
Adds http_header_strictness_test covering the space-before-colon, tab-before-
colon, and obs-fold smuggling vectors plus a well-formed positive control.
…-header-whitespace # Conflicts: # tests/networking_tests/CMakeLists.txt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens
try_parse_requestto reject two malformed header-field forms that are HTTP request-smuggling vectors, per RFC 7230 §3.2.4:Transfer-Encoding : chunkedor a tab before the colon.Both are now answered with
400 Bad Request+ connection close.Why
The header parser lowercases names and looks them up exactly (
headers.find("transfer-encoding"),find("content-length")). It previously stored names without trimming or rejecting malformed whitespace, so an obfuscated header was stored under a key the lookup misses:Transfer-Encoding : chunked→ key"transfer-encoding "(trailing space)Transfer-Encoding\t: chunked→ key"transfer-encoding\t""\ttransfer-encoding"In every case the server sees no
Transfer-Encoding/Content-Length, frames the body as empty, and leaves the encoded payload in the keep-alive buffer, where it is reparsed as a second, smuggled request. A front-end proxy that is tolerant of folding/whitespace frames the body as chunked and forwards the lot as one request — a classic TE/CL desync.This complements #2654, which rejects a clean
Transfer-Encodingheader. Each closes a different half of the same class:Transfer-Encoding: chunkedTransfer-Encoding : chunked(space)Transfer-Encoding\t: chunked(tab)Rejecting (rather than trimming) is deliberate: silently honoring
Content-Length : 5where a peer might frame it differently would just reintroduce the ambiguity. Closing the connection discards the leftover bytes (same safe path the existing 400 responses use). obs-fold is deprecated and not emitted by conforming clients, so false-positive risk is negligible.Testing
New
http_header_strictness_test(binds to an ephemeral port and reads it back viaserver.port()) covers all three smuggling vectors — asserting400and that the smuggledGET /smugglednever reaches a handler — plus a positive control proving well-formed requests with spaces/colons in header values still route and return200.Verified locally (Release + sanitizers):
http_header_strictness_test: 4 tests / 8 asserts pass.http_server_post_test,http_server_api_tests,http_chunked_test(59/233),http_router_test(21/81),cors_test(5/13) all pass.