reqs: remove trailing whitespace from header names - #624
Merged
Merged
Conversation
a header line like "Content-Length : 42" was split into "Content-Length " and "42", leaving the trailing whitespace and allowing an attacker to forward multiple content-length or transfer-encoding headers, circumventing the existing request-smuggling guards. closes tinyproxy#623
There was a problem hiding this comment.
Pull request overview
This PR hardens Tinyproxy’s request header parsing to prevent bypassing existing request-smuggling guards by inserting whitespace before the : in header lines (e.g., Content-Length : 42), addressing #623.
Changes:
- Trims trailing SP/HTAB from the header name immediately before the colon so de-duplication matches
Content-Length:andContent-Length :. - Simplifies header-line length handling by no longer recomputing
lenafter splitting the header.
Suppressed comments (1)
src/reqs.c:662
- This security-hardening path (normalizing whitespace before the colon so CL/TE de-dup works) doesn’t appear to be covered by the existing CI test suite.
make testrunstests/scripts/run_tests.sh, which usestests/scripts/webclient.pl; that client currently can’t send custom raw header lines (no option beyond method/version/entity), so there’s no regression test for the "Content-Length : 100" / "Transfer-Encoding : chunked" bypass described in #623.
Consider adding an integration regression test that sends headers containing whitespace before : and asserts they are normalized (or rejected) such that duplicates are not forwarded.
/* prevent multiple CL/TE headers from being inserted */
if (check_duplicate_header(hashofheaders, header, "content-length") ||
check_duplicate_header(hashofheaders, header, "transfer-encoding"))
return 0;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
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.
a header line like "Content-Length : 42" was split into "Content-Length " and "42", leaving the trailing whitespace and allowing an attacker to forward multiple content-length or transfer-encoding headers, circumventing the existing request-smuggling guards.
closes #623