hysteria2: harden UDP datagram parsing against malformed input#145
Open
zytakeshi wants to merge 1 commit into
Open
hysteria2: harden UDP datagram parsing against malformed input#145zytakeshi wants to merge 1 commit into
zytakeshi wants to merge 1 commit into
Conversation
A crafted UDP datagram could trigger an assert! on the datagram size and an out-of-range slice when reading the varint-prefixed fragment address. Replace the assert! with a graceful return and bounds-check the fragment address length against the remaining buffer, so malformed input is dropped instead of panicking the server.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two small robustness fixes in
run_udp_remote_to_local_loop's parsing of attacker-controlled hysteria2 UDP datagrams. No behavior change for valid datagrams.Changes
assert!on datagram size -> graceful return.max_datagram_sizeis reported by the remote QUIC endpoint and can be attacker-influenced. A value at or belowheader_overheadpreviously hit anassert!(and would also underflow the subsequentmax_datagram_size - header_overhead). It now returns anio::Errorfor that connection instead of panicking.Bounds-check the varint address-length read. When the length indicator selects a multi-byte varint, the code sliced
&data[9..9 + (num_bytes - 1)]without confirming those bytes were present. A small/truncated datagram could make the varint claim more bytes than exist, slicing out of range and panicking. It now checks the remaining buffer length and, per the reference (malformed datagrams are ignored), skips the datagram instead of crashing.Testing
cargo checkpasses (only pre-existing warnings unrelated to this change).