Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 11 additions & 2 deletions src/h2/stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -1364,15 +1364,24 @@ def _initialize_content_length(self, headers: Iterable[Header]) -> None:
self._expected_content_length = 0
return

content_lengths = []

for n, v in headers:
if n == b"content-length":
try:
self._expected_content_length = int(v, 10)
content_lengths.append(int(v, 10))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this lead to a DoS if a malicous payload keeps sending new differing content-length headers and this list keeps growing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think so, it'll be discarded once _initialize_content_length() exits, but I think I have a better idea where only comparing the lengths can work as well, lemme push a fix

except ValueError as err:
msg = f"Invalid content-length header: {v!r}"
raise ProtocolError(msg) from err

return
if not content_lengths:
return

if len(set(content_lengths)) != 1:
msg = "Conflicting content-length headers"
raise ProtocolError(msg)

self._expected_content_length = content_lengths[0]

def _track_content_length(self, length: int, end_stream: bool) -> None:
"""
Expand Down
79 changes: 77 additions & 2 deletions tests/test_invalid_content_lengths.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,24 @@ class TestInvalidContentLengths:
peer is not valid.
"""

example_request_headers = [
example_request_headers_without_content_length = [
(":authority", "example.com"),
(":path", "/"),
(":scheme", "https"),
(":method", "POST"),
]
example_request_headers = [
*example_request_headers_without_content_length,
("content-length", "15"),
]
example_request_headers_bytes = [
example_request_headers_bytes_without_content_length = [
(b":authority", b"example.com"),
(b":path", b"/"),
(b":scheme", b"https"),
(b":method", b"POST"),
]
example_request_headers_bytes = [
*example_request_headers_bytes_without_content_length,
(b"content-length", b"15"),
]
example_response_headers = [
Expand All @@ -39,6 +45,75 @@ class TestInvalidContentLengths:
]
server_config = h2.config.H2Configuration(client_side=False)

@pytest.mark.parametrize(
"request_headers",
[
example_request_headers_without_content_length,
example_request_headers_bytes_without_content_length,
],
)
def test_duplicate_matching_content_lengths(self, frame_factory, request_headers) -> None:
"""
Remote peers sending duplicate matching content-length fields are
accepted.
"""
c = h2.connection.H2Connection(config=self.server_config)
c.initiate_connection()
c.receive_data(frame_factory.preamble())
c.clear_outbound_data_buffer()

headers = frame_factory.build_headers_frame(
headers=[
*request_headers,
("content-length", "15"),
("content-length", "15"),
],
)
data = frame_factory.build_data_frame(
data=b"\x01"*15,
flags=["END_STREAM"],
)

events = c.receive_data(headers.serialize() + data.serialize())

assert isinstance(events[0], h2.events.RequestReceived)
assert isinstance(events[1], h2.events.DataReceived)
assert isinstance(events[2], h2.events.StreamEnded)
assert c.data_to_send() == b""

@pytest.mark.parametrize(
"request_headers",
[
example_request_headers_without_content_length,
example_request_headers_bytes_without_content_length,
],
)
def test_duplicate_conflicting_content_lengths(self, frame_factory, request_headers) -> None:
"""
Remote peers sending duplicate conflicting content-length fields cause
Protocol Errors.
"""
c = h2.connection.H2Connection(config=self.server_config)
c.initiate_connection()
c.receive_data(frame_factory.preamble())
c.clear_outbound_data_buffer()

headers = frame_factory.build_headers_frame(
headers=[
*request_headers,
("content-length", "15"),
("content-length", "16"),
],
)
with pytest.raises(h2.exceptions.ProtocolError):
c.receive_data(headers.serialize())

expected_frame = frame_factory.build_goaway_frame(
last_stream_id=1,
error_code=h2.errors.ErrorCodes.PROTOCOL_ERROR,
)
assert c.data_to_send() == expected_frame.serialize()

@pytest.mark.parametrize("request_headers", [example_request_headers, example_request_headers_bytes])
def test_too_much_data(self, frame_factory, request_headers) -> None:
"""
Expand Down