diff --git a/aiohttp/_http_parser.pyx b/aiohttp/_http_parser.pyx index f7c393ed42a..df1a7437eee 100644 --- a/aiohttp/_http_parser.pyx +++ b/aiohttp/_http_parser.pyx @@ -384,6 +384,13 @@ cdef class HttpParser: name = find_header(self._raw_name) value = self._raw_value.decode('utf-8', 'surrogateescape') + # reject null bytes in header values - matches the Python parser + # check at http_parser.py. llhttp in lenient mode doesn't reject + # these itself, so we need to catch them here. + # ref: RFC 9110 section 5.5 (CTL chars forbidden in field values) + if "\x00" in value: + raise InvalidHeader(self._raw_value) + self._headers.append((name, value)) if len(self._headers) > self._max_headers: raise BadHttpMessage("Too many headers received") diff --git a/aiohttp/_http_writer.pyx b/aiohttp/_http_writer.pyx index 7989c186c89..ee8dcd7d2e9 100644 --- a/aiohttp/_http_writer.pyx +++ b/aiohttp/_http_writer.pyx @@ -111,9 +111,9 @@ cdef inline int _write_str_raise_on_nlcr(Writer* writer, object s): out_str = str(s) for ch in out_str: - if ch == 0x0D or ch == 0x0A: + if ch in {0x0D, 0x0A, 0x00}: raise ValueError( - "Newline or carriage return detected in headers. " + "Newline, carriage return, or null byte detected in headers. " "Potential header injection attack." ) if _write_utf8(writer, ch) < 0: @@ -131,7 +131,7 @@ def _serialize_headers(str status_line, headers): _init_writer(&writer, buf) try: - if _write_str(&writer, status_line) < 0: + if _write_str_raise_on_nlcr(&writer, status_line) < 0: raise if _write_byte(&writer, b'\r') < 0: raise diff --git a/aiohttp/http_writer.py b/aiohttp/http_writer.py index f1bafff86f4..d8fca34ee84 100644 --- a/aiohttp/http_writer.py +++ b/aiohttp/http_writer.py @@ -364,15 +364,16 @@ async def drain(self) -> None: def _safe_header(string: str) -> str: - if "\r" in string or "\n" in string: + if "\r" in string or "\n" in string or "\x00" in string: raise ValueError( - "Newline or carriage return detected in headers. " + "Newline, carriage return, or null byte detected in headers. " "Potential header injection attack." ) return string def _py_serialize_headers(status_line: str, headers: "CIMultiDict[str]") -> bytes: + _safe_header(status_line) headers_gen = (_safe_header(k) + ": " + _safe_header(v) for k, v in headers.items()) line = status_line + "\r\n" + "\r\n".join(headers_gen) + "\r\n\r\n" return line.encode("utf-8") diff --git a/aiohttp/web_exceptions.py b/aiohttp/web_exceptions.py index 19cfdb0657d..782a4d39507 100644 --- a/aiohttp/web_exceptions.py +++ b/aiohttp/web_exceptions.py @@ -99,8 +99,8 @@ def __init__( ) -> None: if reason is None: reason = self.default_reason - elif "\n" in reason: - raise ValueError("Reason cannot contain \\n") + elif "\r" in reason or "\n" in reason: + raise ValueError("Reason cannot contain \\r or \\n") if text is None: if not self.empty_body: diff --git a/aiohttp/web_response.py b/aiohttp/web_response.py index c88911c4dcc..387736509b9 100644 --- a/aiohttp/web_response.py +++ b/aiohttp/web_response.py @@ -157,8 +157,8 @@ def _set_status(self, status: int, reason: str | None) -> None: self._status = status if reason is None: reason = REASON_PHRASES.get(self._status, "") - elif "\n" in reason: - raise ValueError("Reason cannot contain \\n") + elif "\r" in reason or "\n" in reason: + raise ValueError("Reason cannot contain \\r or \\n") self._reason = reason @property diff --git a/tests/autobahn/server/fuzzingclient.json b/tests/autobahn/server/fuzzingclient.json index bf34731a2e8..0842cfcb887 100644 --- a/tests/autobahn/server/fuzzingclient.json +++ b/tests/autobahn/server/fuzzingclient.json @@ -3,11 +3,7 @@ "outdir": "./reports/servers", "servers": [ - { - "agent": "AutobahnServer", - "url": "ws://localhost:9001", - "options": {"version": 18} - } + {"agent": "AutobahnServer", "url": "ws://localhost:9001"} ], "cases": ["*"], diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index e237aad6a88..28fc3aac03d 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -1233,6 +1233,13 @@ def test_http_response_parser_strict_headers(response: HttpResponseParser) -> No response.feed_data(b"HTTP/1.1 200 test\r\nFoo: abc\x01def\r\n\r\n") +def test_http_response_parser_null_byte_in_header_value( + response: HttpResponseParser, +) -> None: + with pytest.raises(http_exceptions.InvalidHeader): + response.feed_data(b"HTTP/1.1 200 OK\r\nFoo: abc\x00def\r\n\r\n") + + def test_http_response_parser_bad_crlf(response: HttpResponseParser) -> None: """Still a lot of dodgy servers sending bad requests like this.""" messages, upgrade, tail = response.feed_data( diff --git a/tests/test_http_writer.py b/tests/test_http_writer.py index 92c2d350a34..e2596ea2e96 100644 --- a/tests/test_http_writer.py +++ b/tests/test_http_writer.py @@ -1061,10 +1061,22 @@ def test_serialize_headers_raises_on_new_line_or_carriage_return(char: str) -> N with pytest.raises( ValueError, - match=( - "Newline or carriage return detected in headers. " - "Potential header injection attack." - ), + match="detected in headers", + ): + _serialize_headers(status_line, headers) + + +def test_serialize_headers_raises_on_null_byte() -> None: + status_line = "HTTP/1.1 200 OK" + headers = CIMultiDict( + { + hdrs.CONTENT_TYPE: "text/plain\x00", + } + ) + + with pytest.raises( + ValueError, + match="null byte detected in headers", ): _serialize_headers(status_line, headers) diff --git a/tests/test_web_exceptions.py b/tests/test_web_exceptions.py index 9371f860af8..b29de08b170 100644 --- a/tests/test_web_exceptions.py +++ b/tests/test_web_exceptions.py @@ -185,9 +185,17 @@ def test_ctor_all(self) -> None: assert resp.status == 200 def test_multiline_reason(self) -> None: - with pytest.raises(ValueError, match=r"Reason cannot contain \\n"): + with pytest.raises(ValueError, match=r"Reason cannot contain"): web.HTTPOk(reason="Bad\r\nInjected-header: foo") + def test_reason_with_cr(self) -> None: + with pytest.raises(ValueError, match=r"Reason cannot contain"): + web.HTTPOk(reason="OK\rSet-Cookie: evil=1") + + def test_reason_with_lf(self) -> None: + with pytest.raises(ValueError, match=r"Reason cannot contain"): + web.HTTPOk(reason="OK\nSet-Cookie: evil=1") + def test_pickle(self) -> None: resp = web.HTTPOk( headers={"X-Custom": "value"}, diff --git a/tests/test_web_response.py b/tests/test_web_response.py index 76d52cca1aa..5e8e2911bda 100644 --- a/tests/test_web_response.py +++ b/tests/test_web_response.py @@ -934,6 +934,27 @@ def test_set_status_with_empty_reason() -> None: assert resp.reason == "" +def test_set_status_reason_with_cr() -> None: + resp = web.StreamResponse() + + with pytest.raises(ValueError, match="Reason cannot contain"): + resp.set_status(200, "OK\rSet-Cookie: evil=1") + + +def test_set_status_reason_with_lf() -> None: + resp = web.StreamResponse() + + with pytest.raises(ValueError, match="Reason cannot contain"): + resp.set_status(200, "OK\nSet-Cookie: evil=1") + + +def test_set_status_reason_with_crlf() -> None: + resp = web.StreamResponse() + + with pytest.raises(ValueError, match="Reason cannot contain"): + resp.set_status(200, "OK\r\nSet-Cookie: evil=1") + + async def test_start_force_close() -> None: req = make_request("GET", "/") resp = web.StreamResponse() @@ -1236,7 +1257,7 @@ async def test_render_with_body(buf: bytearray, writer: AbstractStreamWriter) -> async def test_multiline_reason(buf: bytearray, writer: AbstractStreamWriter) -> None: - with pytest.raises(ValueError, match=r"Reason cannot contain \\n"): + with pytest.raises(ValueError, match=r"Reason cannot contain \\r or \\n"): web.Response(reason="Bad\r\nInjected-header: foo")