Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/13136.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed Python parser not rejecting a bare ``LF`` in the request line -- by :user:`Dreamsorcerer`.
11 changes: 9 additions & 2 deletions THREAT_MODEL.md
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ into `StreamReader`) is then handed to `web_protocol.RequestHandler` and
| 1.8 | `Transfer-Encoding` lenience | `_is_chunked_te` requires `chunked` to be the last value; duplicate `chunked` rejected (`#10611`). Request parser strict. | None. |
| 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]` for line errors. Server-side error path renders 4xx with the exception message; tracebacks only when `DEBUG=True`. | **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*`). | None. When new attack vectors emerge, add them to the parameterised tests. |
| 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.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.** |

Expand Down Expand Up @@ -324,6 +324,13 @@ into `StreamReader`) is then handed to `web_protocol.RequestHandler` and
could grow without bound. Memory was previously bounded only
transitively by `read_bufsize` ([§5.7](#57-server-connection-lifecycle) threat 7.4);
the fix adds an explicit pipeline-count cap.
- **PR #13136** (3.14.2) — the pure-Python request parser buffered a
bare `LF` line ending (where `CRLF` is required) across reads instead of
rejecting it, so bytes from a following request on the same connection
could be concatenated into the failed parse and disclosed in the error.
Now rejected at the point it is seen, matching llhttp (request line,
headers, chunk-size, trailers). Not a vulnerability because no proxy would
pipeline requests from multiple clients and forward raw LFs to the backend.

These are all currently in place; this section assumes no regression.

Expand Down
17 changes: 17 additions & 0 deletions aiohttp/http_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,11 @@ def get_content_length() -> int | None:
should_close = msg.should_close
else:
self._tail = data[start_pos:]
# A bare LF here means CRLF was required:
# reject instead of buffering, else a following request's
# bytes get appended to this line and leak in the error.
if b"\n" in self._tail:
raise BadHttpMessage("Bad line ending, expected CRLF")
if len(self._tail) > self.max_line_size:
raise LineTooLong(self._tail[:100] + b"...", self.max_line_size)
data = EMPTY
Expand Down Expand Up @@ -1008,6 +1013,12 @@ def feed_data(
self._chunk_size = size
self.payload.begin_http_chunk_receiving()
else:
if b"\n" in chunk:
exc = TransferEncodingError(
"Bad chunk-size line ending, expected CRLF"
)
set_exception(self.payload, exc)
raise exc
self._chunk_tail = chunk
return PayloadState.PAYLOAD_NEEDS_INPUT, b""

Expand Down Expand Up @@ -1052,6 +1063,12 @@ def feed_data(
if self._chunk == ChunkState.PARSE_TRAILERS:
pos = chunk.find(SEP)
if pos < 0: # No line found
if b"\n" in chunk:
exc = TransferEncodingError(
"Bad trailer line ending, expected CRLF"
)
set_exception(self.payload, exc)
raise exc
self._chunk_tail = chunk
return PayloadState.PAYLOAD_NEEDS_INPUT, b""

Expand Down
2 changes: 1 addition & 1 deletion requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ uvloop==0.22.1 ; platform_system != "Windows"
# -r requirements/lint.in
valkey==6.1.1
# via -r requirements/lint.in
virtualenv==21.6.0
virtualenv==21.6.1
# via pre-commit
wait-for-it==2.3.0
# via -r requirements/test-common-base.in
Expand Down
2 changes: 1 addition & 1 deletion requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ uvloop==0.22.1 ; platform_system != "Windows" and implementation_name == "cpytho
# -r requirements/lint.in
valkey==6.1.1
# via -r requirements/lint.in
virtualenv==21.6.0
virtualenv==21.6.1
# via pre-commit
wait-for-it==2.3.0
# via -r requirements/test-common-base.in
Expand Down
2 changes: 1 addition & 1 deletion requirements/lint.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ uvloop==0.22.1 ; platform_system != "Windows"
# via -r requirements/lint.in
valkey==6.1.1
# via -r requirements/lint.in
virtualenv==21.6.0
virtualenv==21.6.1
# via pre-commit
yarl==1.24.2
# via aiohttp
Expand Down
35 changes: 35 additions & 0 deletions tests/test_http_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,41 @@ def test_invalid_linebreak(
parser.feed_data(text)


def test_partial_request_split_still_parses(parser: HttpRequestParser) -> None:
"""A legitimate request split mid-line must still stream."""
messages, _, _ = parser.feed_data(b"GET /split HTTP/1.1\r\nHo")
assert not messages
messages, _, _ = parser.feed_data(b"st: a\r\n\r\n")
assert len(messages) == 1
assert messages[0][0].path == "/split"


@pytest.mark.parametrize(
"attacker",
(
pytest.param(
b"GET /attacker HTTP/1.1\nHost: a\nX-Foo: bar\n\n", id="request_line"
),
pytest.param(
b"POST /attacker HTTP/1.1\r\nHost: a\r\n"
b"Transfer-Encoding: chunked\r\n\r\n5\n",
id="chunk_size",
),
pytest.param(
b"POST /attacker HTTP/1.1\r\nHost: a\r\n"
b"Transfer-Encoding: chunked\r\n\r\n0\r\nX-Trailer: bad\n",
id="trailer",
),
),
)
def test_reject_bare_lf_at_point_seen(
parser: HttpRequestParser, attacker: bytes
) -> None:
"""A bare LF where CRLF is required is rejected at the point it's seen."""
with pytest.raises(http_exceptions.BadHttpMessage):
parser.feed_data(attacker)


def test_cve_2023_37276(parser: HttpRequestParser) -> None:
text = b"""POST / HTTP/1.1\r\nHost: localhost:8080\r\nX-Abc: \rxTransfer-Encoding: chunked\r\n\r\n"""
with pytest.raises(http_exceptions.BadHttpMessage):
Expand Down
Loading