Skip to content

Reject non-numeric Content-Length header values#837

Open
apoorvdarshan wants to merge 1 commit into
cherrypy:mainfrom
apoorvdarshan:fix/issue-738-content-length-underscore
Open

Reject non-numeric Content-Length header values#837
apoorvdarshan wants to merge 1 commit into
cherrypy:mainfrom
apoorvdarshan:fix/issue-738-content-length-underscore

Conversation

@apoorvdarshan

Copy link
Copy Markdown

Fixes #738

Root cause

When reading request headers, HTTPRequest.read_request_headers() parsed the
Content-Length value with a bare int():

cl = int(self.inheaders.get(b'Content-Length', 0))

int() is more permissive than the Content-Length grammar, which is
1*DIGIT per RFC 9110 §8.6.
In particular int() silently accepts digit-separating underscores
(int('1_0') == 10), a leading +/- sign, and surrounding whitespace. The
existing try/except ValueError only caught values that int() itself
rejects (e.g. not-an-integer), so malformed-but-parseable values slipped
through 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:

import socket, threading
from cheroot import wsgi

def app(environ, start_response):
    n = int(environ.get('CONTENT_LENGTH', 0) or 0)
    body = environ['wsgi.input'].read(n)
    start_response('200 OK', [('Content-Type', 'text/plain')])
    return [b'read %d bytes' % len(body)]

server = wsgi.Server(('127.0.0.1', 0), app)
server.prepare()
host, port = server.bind_addr
threading.Thread(target=server.serve, daemon=True).start()

def send(cl):
    s = socket.create_connection((host, port), timeout=2)
    s.sendall(
        b'POST /u HTTP/1.1\r\nHost: %s\r\nContent-Length: %s\r\n'
        b'Connection: close\r\n\r\nAB' % (host.encode(), cl)
    )
    data = b''
    try:
        while True:
            chunk = s.recv(4096)
            if not chunk:
                break
            data += chunk
    except socket.timeout:
        pass
    s.close()
    print('Content-Length=%-6r ->' % cl, data.split(b'\r\n', 1)[0])

for cl in (b'2', b'1_0', b'+2'):
    send(cl)

Wrong output (before the fix):

Content-Length=b'2'   -> b'HTTP/1.1 200 OK'
Content-Length=b'1_0' -> b''
Content-Length=b'+2'  -> b'HTTP/1.1 200 OK'

1_0 was parsed as 10; the server then blocked waiting for 10 body bytes
that never arrived (only 2 were sent), so no response came back before the
client timed out. +2 was parsed as 2 and accepted outright. Both should
have been rejected.

Corrected output (after the fix):

Content-Length=b'2'   -> b'HTTP/1.1 200 OK'
Content-Length=b'1_0' -> b'HTTP/1.1 400 Bad Request'
Content-Length=b'+2'  -> b'HTTP/1.1 400 Bad Request'

Fix

Validate the raw header value against [0-9]+ (a module-level compiled
NUMERIC_REGEX) before calling int(), and reuse the existing
400 Bad Request / Malformed Content-Length Header. response path when it
does not match. The change is confined to the one parse site in
read_request_headers(). The second int(Content-Length) in respond() is
reached only after read_request_headers() has already returned successfully,
so it never sees an unvalidated value. A default of b'0' preserves the
existing behavior for requests without a Content-Length header.

Tests

Parametrized the existing test_Content_Length_not_int in
cheroot/test/test_conn.py to also cover 1_0, +10 and 0x10. On the
unmodified code the 1_0 and +10 cases fail (the server returns 408
after blocking on the never-arriving body instead of 400); with the fix all
four cases return 400.

$ python -m pytest cheroot/test/test_conn.py::test_Content_Length_not_int
4 passed

The surrounding suites pass as well (test_conn.py, test_core.py,
test_wsgi.py, test_server.py), and flake8 reports no new offenses on the
changed files. A changelog fragment (docs/changelog-fragments.d/738.bugfix.rst)
is included.

Disclosure: prepared with AI assistance; reviewed and verified locally.

`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
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided A mark meaning that a new change log entry is present within the patch. label Jul 9, 2026
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.16%. Comparing base (3937fe1) to head (9b2a3b6).
✅ All tests successful. No failed tests found.

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     

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided A mark meaning that a new change log entry is present within the patch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_ is incorrectly permitted in Content-Length header values

1 participant