reject negative 64-bit payload length in WebSocketFrameReader#854
Open
dxbjavid wants to merge 1 commit into
Open
reject negative 64-bit payload length in WebSocketFrameReader#854dxbjavid wants to merge 1 commit into
dxbjavid wants to merge 1 commit into
Conversation
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.
readFrame accumulates the 127-form extended payload length into a signed long, but never checks that the most significant bit is clear the way RFC 6455 section 5.2 requires. A masked client frame declaring a length of 0xFFFFFFFF00000000 leaves len negative, so both the Integer.MAX_VALUE and the maxFramePayloadSize guards pass, and the value is then narrowed with a cast when the payload array is allocated. The reader hands back a frame whose payload length has nothing to do with the declared one, and carries on parsing the bytes that follow as the next frame header, so a peer can desynchronise the frame stream; 0xFFFFFFFFFFFFFFFF narrows to -1 and throws NegativeArraySizeException instead.
The client-side WebSocketFrameDecoder already rejects a negative extended length in this spot, so this is really just the same guard missing on the server-side reader, placed before the narrowing rather than after it. Added a test that feeds the reader such a header; it passes on the current code because nothing is thrown.