Bounds-check unchecked input reads in fastlz1/fastlz2_decompress (OOB read) - #12
Open
eobi wants to merge 1 commit into
Open
Bounds-check unchecked input reads in fastlz1/fastlz2_decompress (OOB read)#12eobi wants to merge 1 commit into
eobi 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
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.
Bound-check the two unchecked input reads in the decompressors
fastlz_decompress()is documented as "crash-proof against corrupted and/or malicious data",and the decoders enforce that with the always-on
FASTLZ_BOUND_CHECKmacro. Two input reads aremissing that check, so a crafted/truncated stream reads one byte past the end of the input buffer
(CWE-125). A third read happens before the length is validated.
fastlz2_decompress— thecode = *ip++;right after theif (len == 7-1) do { ... } while (code == 255);loop has no bound check (it is the only one of the function's input reads withoutone). A 3-byte input
20 ff 38reaches it withipat the end of the buffer:fastlz1_decompress— the analogousref -= *ip++;after itsif (len == 7-1) { ...; len += *ip++; }block is likewise unchecked.fastlz_decompress— reads the level byte*(const uint8_t*)inputbefore checkinglength >= 1, so a zero-length call over-reads by one byte.Fix
Three lines, using the existing
FASTLZ_BOUND_CHECKmacro (this PR).Verification
cc -g -fsanitize=address poc.c fastlz.c && ./poc, PoC in the linked report): faultsbefore, clean after.
Found with a coverage-guided libFuzzer + ASan harness; happy to share it.