Reject non-numeric Content-Length header values#837
Open
apoorvdarshan wants to merge 1 commit into
Open
Conversation
`int()` silently accepts strings that are not valid `1*DIGIT` values, such
as `1_0` (a digit-separating underscore, `int('1_0') == 10`), `+10` (a
leading sign) and values with surrounding whitespace. None of these are
permitted by the `Content-Length` grammar in RFC 9110 section 8.6, yet
cheroot parsed them and used the resulting length, so a request with
`Content-Length: 1_0` was accepted instead of being rejected.
Validate the raw header value against `[0-9]+` before parsing it with
`int()` and respond with `400 Bad Request` when it does not match, reusing
the existing malformed-Content-Length response path.
Fixes cherrypy#738
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #837 +/- ##
==========================================
- Coverage 78.19% 78.16% -0.04%
==========================================
Files 41 41
Lines 4788 4790 +2
Branches 547 548 +1
==========================================
Hits 3744 3744
- Misses 905 906 +1
- Partials 139 140 +1 |
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.
Fixes #738
Root cause
When reading request headers,
HTTPRequest.read_request_headers()parsed theContent-Lengthvalue with a bareint():int()is more permissive than theContent-Lengthgrammar, which is1*DIGITper RFC 9110 §8.6.In particular
int()silently accepts digit-separating underscores(
int('1_0') == 10), a leading+/-sign, and surrounding whitespace. Theexisting
try/except ValueErroronly caught values thatint()itselfrejects (e.g.
not-an-integer), so malformed-but-parseable values slippedthrough and were used as the body length.
Reproduction
Minimal WSGI server that echoes the received body length, sending a POST with a
2-byte body but a malformed
Content-Length:Wrong output (before the fix):
1_0was parsed as10; the server then blocked waiting for 10 body bytesthat never arrived (only 2 were sent), so no response came back before the
client timed out.
+2was parsed as2and accepted outright. Both shouldhave been rejected.
Corrected output (after the fix):
Fix
Validate the raw header value against
[0-9]+(a module-level compiledNUMERIC_REGEX) before callingint(), and reuse the existing400 Bad Request/Malformed Content-Length Header.response path when itdoes not match. The change is confined to the one parse site in
read_request_headers(). The secondint(Content-Length)inrespond()isreached only after
read_request_headers()has already returned successfully,so it never sees an unvalidated value. A default of
b'0'preserves theexisting behavior for requests without a
Content-Lengthheader.Tests
Parametrized the existing
test_Content_Length_not_intincheroot/test/test_conn.pyto also cover1_0,+10and0x10. On theunmodified code the
1_0and+10cases fail (the server returns408after blocking on the never-arriving body instead of
400); with the fix allfour cases return
400.The surrounding suites pass as well (
test_conn.py,test_core.py,test_wsgi.py,test_server.py), andflake8reports no new offenses on thechanged files. A changelog fragment (
docs/changelog-fragments.d/738.bugfix.rst)is included.
Disclosure: prepared with AI assistance; reviewed and verified locally.