From 956f140095b21a88e11def934d42de85cc0b2b6b Mon Sep 17 00:00:00 2001 From: arshsmith Date: Sun, 23 Aug 2026 21:40:01 +0530 Subject: [PATCH 1/2] Reject control characters in the request target (#13212) --- CHANGES/13212.bugfix.rst | 3 +++ THREAT_MODEL.md | 2 +- aiohttp/http_parser.py | 7 +++++++ tests/test_http_parser.py | 23 +++++++++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 CHANGES/13212.bugfix.rst diff --git a/CHANGES/13212.bugfix.rst b/CHANGES/13212.bugfix.rst new file mode 100644 index 00000000000..1d8392c6b71 --- /dev/null +++ b/CHANGES/13212.bugfix.rst @@ -0,0 +1,3 @@ +Rejected control characters in the request target in the pure-Python HTTP parser, +matching the llhttp-backed parser, which already refuses them +-- by :user:`arshsmith1`. diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index 5ad9d210368..da8c210b543 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -268,7 +268,7 @@ into `StreamReader`) is then handed to `web_protocol.RequestHandler` and | 1.9 | Chunk-size DoS | The parser doesn't cap chunk size, but **server-side body length is bounded by `client_max_size` (default `1 MiB`)** in `web_request.py:BaseRequest.read`. Client-side responses are bounded by user-supplied `max_body_size` / streaming reads. | None. If a cap is ever needed at the parser level, plumb it through `HttpPayloadParser`. | | 1.10 | Chunk-extension DoS | Chunk-extension content is bounded by the same wire-level size constraints (it shares the chunk-size line with `max_line_size`). | **Add an explicit test that chunk-extension flooding cannot blow past `max_line_size`.** | | 1.11 | Parser error reflection | `http_parser.py` truncates to `[:100]` only for `LineTooLong`; `BadStatusLine` / `InvalidHeader` / `TransferEncodingError` carry the offending line up to `max_line_size` / `max_field_size`. | **Audit any aiohttp path where `BadHttpMessage` content is reflected to the client unsanitised.** **User**: Review custom `web_log` configurations and any middleware that reflects parser exception messages back to the peer. | -| 1.12 | Cython ⇄ pure-Python divergence | `tests/test_http_parser.py` parameterises tests over `REQUEST_PARSERS` / `RESPONSE_PARSERS` (pure-Python always; Cython when the extension imports). The high-leverage attack vectors are already covered under both backends: CL+TE (`test_content_length_transfer_encoding`), CL×N (`test_duplicate_singleton_header_rejected`), obs-fold (`test_reject_obsolete_line_folding`, `test_http_response_parser_obs_line_folding*`), CR/LF/NUL (`test_bad_headers`, `test_http_response_parser_null_byte_in_header_value`, `test_http_response_parser_bad_crlf`), version regex (`test_http_request_parser_bad_version*`, `test_http_response_parser_bad_version*`), bare-LF line endings (`test_reject_bare_lf_no_cross_request_leak`). | None. When new attack vectors emerge, add them to the parameterised tests. | +| 1.12 | Cython ⇄ pure-Python divergence | `tests/test_http_parser.py` parameterises tests over `REQUEST_PARSERS` / `RESPONSE_PARSERS` (pure-Python always; Cython when the extension imports). The high-leverage attack vectors are already covered under both backends: CL+TE (`test_content_length_transfer_encoding`), CL×N (`test_duplicate_singleton_header_rejected`), obs-fold (`test_reject_obsolete_line_folding`, `test_http_response_parser_obs_line_folding*`), CR/LF/NUL (`test_bad_headers`, `test_http_response_parser_null_byte_in_header_value`, `test_http_response_parser_bad_crlf`), version regex (`test_http_request_parser_bad_version*`, `test_http_response_parser_bad_version*`), bare-LF line endings (`test_reject_bare_lf_no_cross_request_leak`), control characters in the request target (`test_http_request_parser_ctl_in_request_target`). | None. When new attack vectors emerge, add them to the parameterised tests. | | 1.13 | llhttp version drift | Manual upgrade via `make generate-llhttp`; vendor pinned in `vendor/llhttp/package.json`. | Track upstream releases (e.g. via Dependabot rule for `vendor/llhttp/package.json`), bump on every llhttp release, regenerate in CI. | | 1.14 | npm-side compromise of `llhttp` | The vendored output is checked into git, so a compromise during a future regen would be detectable in PR review. See [§5.19](#519-build--release-supply-chain). | **Make the llhttp build reproducible: pin Node.js version, commit the npm lockfile, and on every bump verify the regenerated C against upstream's release tarballs before committing.** | diff --git a/aiohttp/http_parser.py b/aiohttp/http_parser.py index 6598517664d..e5ba3d129f2 100644 --- a/aiohttp/http_parser.py +++ b/aiohttp/http_parser.py @@ -86,6 +86,7 @@ _FIELD_VALUE_FORBIDDEN_CTL_RE: Final[Pattern[str]] = re.compile( r"[\x00-\x08\x0a-\x1f\x7f]" ) +_TARGET_FORBIDDEN_CTL_RE: Final[Pattern[str]] = re.compile(r"[\x00-\x1f\x7f]") VERSRE: Final[Pattern[str]] = re.compile(r"HTTP/(\d)\.(\d)", re.ASCII) DIGITS: Final[Pattern[str]] = re.compile(r"\d+", re.ASCII) HEXDIGITS: Final[Pattern[bytes]] = re.compile(rb"[0-9a-fA-F]+") @@ -665,6 +666,12 @@ def parse_message(self, lines: list[bytes]) -> RawRequestMessage: raise BadHttpMethod(method) method = method.upper() + # https://www.rfc-editor.org/rfc/rfc9112#section-3.2-4 + if _TARGET_FORBIDDEN_CTL_RE.search(path): + raise InvalidURLError( + path.encode(errors="surrogateescape").decode("latin1") + ) + # version match = VERSRE.fullmatch(version) if match is None: diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index 259d88f7dfa..701622cd1e4 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -2034,6 +2034,29 @@ def test_http_request_parser_bad_nonascii_uri(parser: HttpRequestParser) -> None parser.feed_data(b"GET \xff HTTP/1.1\r\n\r\n") +@pytest.mark.parametrize( + "target", + ( + b"/a\x00b", + b"/a\tb", + b"/a\nb", + b"/a\rb", + b"/a\x1fb", + b"/a\x7fb", + b"/a?b=\x01", + b"/a#\x01", + b"http://example.com/a\x00b", + ), + ids=("nul", "htab", "lf", "cr", "us", "del", "query", "fragment", "absolute-form"), +) +def test_http_request_parser_ctl_in_request_target( + parser: HttpRequestParser, target: bytes +) -> None: + # https://www.rfc-editor.org/rfc/rfc9112#section-3.2-4 + with pytest.raises(http_exceptions.BadHttpMessage): + parser.feed_data(b"GET " + target + b" HTTP/1.1\r\nHost: a\r\n\r\n") + + @pytest.mark.parametrize("size", [40965, 8191]) def test_http_request_max_status_line(parser: HttpRequestParser, size: int) -> None: path = b"t" * (size - 5) From b2b2bce03c17521dc454cdd4a6e19ab0a08bce6b Mon Sep 17 00:00:00 2001 From: arshsmith Date: Sun, 23 Aug 2026 21:40:41 +0530 Subject: [PATCH 2/2] Fix infinite loop parsing the Forwarded header (#13229) --- CHANGES/13229.bugfix.rst | 3 +++ aiohttp/web_request.py | 8 ++++++-- tests/test_web_request.py | 17 +++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 CHANGES/13229.bugfix.rst diff --git a/CHANGES/13229.bugfix.rst b/CHANGES/13229.bugfix.rst new file mode 100644 index 00000000000..1eeda8a64d8 --- /dev/null +++ b/CHANGES/13229.bugfix.rst @@ -0,0 +1,3 @@ +Fixed an infinite loop in :attr:`aiohttp.web.BaseRequest.forwarded` that let a +single malformed ``Forwarded`` request header (e.g. ``Forwarded: a``) spin the +event loop at 100% CPU and hang the worker -- by :user:`arshsmith1`. diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index b35f6124c53..5f426d743c7 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -377,9 +377,13 @@ def forwarded(self) -> tuple[Mapping[str, str], ...]: value += port elem[name.lower()] = value pos += len(match.group(0)) - elif not field_value[pos : field_value.find(";", pos)].strip(" \t"): + elif (semi := field_value.find(";", pos)) == -1: + # No further pair to parse; a trailing empty or malformed + # value ends this field-value. + break + elif not field_value[pos:semi].strip(" \t"): # Empty value - pos = field_value.find(";", pos) + 1 + pos = semi + 1 else: # bad syntax here, skip to next field value break diff --git a/tests/test_web_request.py b/tests/test_web_request.py index d420495b39c..d1b6f553a99 100644 --- a/tests/test_web_request.py +++ b/tests/test_web_request.py @@ -792,6 +792,23 @@ def test_single_forwarded_header_injection2() -> None: assert req.forwarded[1]["for"] == "_real" +@pytest.mark.parametrize( + "header, expected", + [ + ("a", {}), + ("; a", {}), + ("for=1.2.3.4; a", {"for": "1.2.3.4"}), + ("for=_real; x", {"for": "_real"}), + ("bad; for=_real", {}), + ], +) +def test_single_forwarded_header_trailing_bad_value( + header: str, expected: dict[str, str] +) -> None: + req = make_mocked_request("GET", "/", headers=CIMultiDict({"Forwarded": header})) + assert dict(req.forwarded[0]) == expected + + def test_single_forwarded_header_long_quoted_string() -> None: header = 'for="' + "\\\\" * 5000 + '"' req = make_mocked_request("GET", "/", headers=CIMultiDict({"Forwarded": header}))