fix: prevent heap overflow in Q decompression#1
Open
belowzeroff wants to merge 1 commit into
Open
Conversation
q_decompress() sized the output buffer from the frame's declared uncompressed size but never checked that a back-reference's expansion (2 literal bytes + up to 255 copied bytes) stayed within it. A malformed or hostile frame can declare a tiny out_size yet emit opcodes that write past the allocation — a heap buffer overflow. This is remotely reachable on the server: q_read_body() passes the client-controlled `compressed` header flag straight into q_decode(), so a peer can request the decompression path with a crafted body. Read the back-reference index and length first, then reject the frame if `s + 2 + n` would exceed out_size, instead of writing past `result`. The source reads are already in bounds (r < s <= out_size). Verified with an AddressSanitizer harness: the crafted frame trips ASan before the change and is rejected cleanly after.
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
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.
Problem
q_decompress()(q.c) sizes the output buffer from the frame's declareduncompressed size, but never checks that a back-reference's expansion — 2
literal bytes plus up to 255 copied bytes — stays within it. A malformed or
hostile frame can declare a tiny
out_sizeyet emit opcodes that write past theallocation: a heap buffer overflow.
This is remotely reachable on the server.
q_read_body()inq_server.cpasses the client-controlled
compressedheader flag straight intoq_decode(), so a peer can select the decompression path with a crafted body.The
rayforce -qserver also performs no authentication, so the path ispre-auth.
Fix
Read the back-reference index and length first, then reject the frame when
s + 2 + nwould exceedout_size, instead of writing pastresult. Thesource reads are already in bounds (
r < s <= out_size), so only the outputwrite needed guarding. Valid frames are unaffected — the write sequence is
unchanged, only an out-of-bounds expansion is now refused.
Validation
Reproduced with an AddressSanitizer harness driving
q_decompressdirectly(the function has no rayforce dependencies). A frame declaring
out_size = 2with a back-reference of
n = 255:AddressSanitizer: heap-buffer-overflow ... in q_decompressrc = -1), no out-of-bounds access.The full server test suite (
make test, server legs) still passes 32/32.🤖 Generated with Claude Code