Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject Content-Length longer than 1 billion TB #181

Merged

Conversation

Julien00859
Copy link
Contributor

@Julien00859 Julien00859 commented Jan 11, 2025

So there is this CVE on CPython https://www.cve.org/CVERecord?id=CVE-2020-10735, the problem is that parsing a base-10 integer with int() is quadratic. A rogue agent can forge a bad Content-Length header with a huge integer to DOS h11.

The way the bug was solved in CPython 3.11 was to verify the length of the int input, to make sure it stayed under a reasonable limit (4300 digits by default).

So using h11 with Py3.11, we get a ValueError when we try to parses a rogue Content-Length. But in Py3.8.2 (the lowest I have on my machine) it takes ages to parse it.

Consider the folowing example:

import h11

conn = h11.Connection(h11.CLIENT)
conn.send(h11.Request(method="GET", target="/", headers=[("Host", "example.com")]))
conn.send(h11.EndOfMessage())

response = bytearray(b"HTTP/1.1 200 OK\r\nContent-Length: ")
response += b"1" * 1_000_000
response += b"\r\n\r\n"

conn.receive_data(response)
conn.next_event()

So we craft a response with a Content-Length with 1MB digits.

The script takes 7 seconds to run:

$ time python3.8.2 /tmp/exploit.py

________________________________________________________
Executed in    7,60 secs    fish           external
   usr time    7,47 secs  638,00 micros    7,47 secs
   sys time    0,07 secs  264,00 micros    0,07 secs

Changing the Content-Length for another header (e.g. Content-Type) we don't have any problem.

$ sed s/Content-Length/Content-Type/ -i /tmp/exploit.py 
$ time python3.8.2 /tmp/exploit.py

________________________________________________________
Executed in  252,35 millis    fish           external
   usr time  151,82 millis    0,00 millis  151,82 millis
   sys time   67,64 millis    1,58 millis   66,07 millis

This PR solves 2 things:

  • It makes sure parsing a rogue Content-Length header doesn't DOS h11.
  • It raises a h11.ProtocolError instead of a ValueError.

@Julien00859 Julien00859 force-pushed the Julien00859/get_int_max_str_digits branch from 76ef74e to 40a14d8 Compare January 11, 2025 01:28
@Julien00859 Julien00859 marked this pull request as ready for review January 11, 2025 01:28
@Julien00859 Julien00859 force-pushed the Julien00859/get_int_max_str_digits branch from 40a14d8 to 8ff7107 Compare January 11, 2025 01:59
@njsmith
Copy link
Member

njsmith commented Jan 11, 2025

Let's make the limit something even more reasonable, like... 20? that's long enough for a 2**64 byte request body.

@Julien00859 Julien00859 force-pushed the Julien00859/get_int_max_str_digits branch from 8ff7107 to dafc1e7 Compare January 11, 2025 14:35
@Julien00859 Julien00859 force-pushed the Julien00859/get_int_max_str_digits branch from dafc1e7 to 60782ad Compare January 11, 2025 14:37
@Julien00859 Julien00859 changed the title Reject Content-Length longer than 4300 digits Reject Content-Length longer than 1 billion TB Jan 11, 2025
@sigmavirus24 sigmavirus24 self-requested a review January 11, 2025 15:24
@njsmith njsmith merged commit 70a96be into python-hyper:master Jan 12, 2025
9 checks passed
@Julien00859 Julien00859 deleted the Julien00859/get_int_max_str_digits branch January 13, 2025 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants