Conversation
Each reserved-skippable chunk made ensureBuffer() recurse into itself, so a crafted stream added one call-stack frame per ~4 input bytes and terminated reads with StackOverflowError. Skipping iteratively keeps the stack depth constant regardless of how many chunks precede the next data frame. Fixes xerial#731 Signed-off-by: Ilias Aberkane <iliasaberkane2908@gmail.com>
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.
Issue
Fixes #731 — uncontrolled recursion (CWE-674, CVSS 5.3) in
SnappyFramedInputStream.ensureBuffer(), reported by August829 with a PoC.Root cause
getFrameMetaData()accepts zero-length reserved-skippable chunks (0x80–0xfe,minLength = 0), and the SKIP branch ofensureBuffer()handled each one by callingreturn ensureBuffer(). Every chunk — only 4 bytes on the wire — therefore consumed one call-stack frame, so a crafted stream of ~78 KB was enough to terminate the decompressing thread withStackOverflowError.Fix
The SKIP handling now consumes runs of skippable chunks in a
whileloop instead of recursing, so stack depth stays constant no matter how many chunks precede the next data frame. Behaviour for all other frame actions is unchanged; the loop exits with the first non-SKIPFrameMetaDataand the existing data path runs as before.Tests
Two regression tests in
SnappyFramedStreamTest:testSkippableChunkRun— 100,000 zero-length skippable chunks followed by a checksummed uncompressed data chunk: the data is returned intact (fails pre-fix withStackOverflowErroratSnappyFramedInputStream.java:531).testSkippableChunkRunToEof— a stream that is nothing but skippable chunks ends in-1instead ofStackOverflowError../sbt "testOnly org.xerial.snappy.SnappyFramedStreamTest": 19/19 pass; the two new tests fail against the unmodified code, confirming they reproduce the reported bug. Full./sbt test: identical result set as unmodified upstream — the only failures are the 11 pre-existingisTooLarge*ArrayInputLengthcases inSnappyTestthat hitOutOfMemoryErroron small test heaps before reaching Snappy code.