Skip to content

Integer overflow when parsing prefixed integers from qpack #301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 3, 2025

Conversation

Ruben2424
Copy link
Contributor

Changes in this PR:

Overflow

previous to this PR the prefix_int MAX_POWER restricted to 10 * 7 (70) bits, which does not fit in an u64.
This PR restricts the MAX_POWER to 9 * 7 (63) bits. This still meets the requirement from the qpack spec of 62 bit:

QPACK implementations MUST be able to decode integers up to and including 62 bits long.

This means our implementation now rejects some values that would fit in u64.
Since the spec only requires 62 bit i chose this approach instead of adding more logic to the decoder.

@Ruben2424 Ruben2424 merged commit 72af1ed into hyperium:master May 3, 2025
16 checks passed
@Ruben2424
Copy link
Contributor Author

@seanmonstar had to change a few things to satisfy CI after your review. Some unrelated to the original change:

I moved the server_and_client_should_connect_successfully rust test which spawned processes to a script which the ci now runs directly. Idk what exactly caused this to fail, but now it works.

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.

2 participants