[pull] master from aio-libs:master

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`.

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`.

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.** |
 

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:

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

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)

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}))