Skip to content

streams: reject STOP_SENDING and MAX_STREAM_DATA beyond the stream limit#2652

Open
c-tonneslan wants to merge 1 commit into
quinn-rs:mainfrom
c-tonneslan:fix/stream-limit-stop-sending-max-stream-data
Open

streams: reject STOP_SENDING and MAX_STREAM_DATA beyond the stream limit#2652
c-tonneslan wants to merge 1 commit into
quinn-rs:mainfrom
c-tonneslan:fix/stream-limit-stop-sending-max-stream-data

Conversation

@c-tonneslan

Copy link
Copy Markdown

Closes #2650.

received_reset runs validate_receive_id, so a RESET_STREAM that references a stream ID past the limit we advertised gets caught as a STREAM_LIMIT_ERROR. received_stop_sending and received_max_stream_data never had the equivalent check, so a peer could send STOP_SENDING or MAX_STREAM_DATA for a bidi stream it was never allowed to open and we'd silently ignore the frame instead of raising a protocol violation.

I pulled the check into validate_remote_stream_limit and call it from both handlers. Locally initiated streams stay bounded by the existing is_local_unopened checks, so this only tightens handling of peer-initiated stream IDs. Added unit tests for both frames covering the within-limit and over-limit cases.

received_reset already runs validate_receive_id, so a RESET_STREAM
referencing a stream ID past the limit we advertised is caught as a
STREAM_LIMIT_ERROR. received_stop_sending and received_max_stream_data
never had an equivalent check, so a peer could send STOP_SENDING or
MAX_STREAM_DATA for a stream it was never allowed to open and we'd just
ignore the frame instead of treating it as a protocol violation.

Add validate_remote_stream_limit and call it from both. Locally
initiated streams are still bounded by the existing is_local_unopened
checks, so this only tightens handling of peer-initiated stream IDs.

Closes quinn-rs#2650

Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>

@Ralith Ralith left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! Unfortunately it looks like you've raced with #2651, which came in earlier. Code looks good, aside from the missed update to the fuzzer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Raise a transport error on out-of-bounds STOP_SENDING and MAX_STREAM_DATA

2 participants