ebp: guard flag-driven reads against truncated input - #184
Open
ChrisJr404 wants to merge 1 commit into
Open
Conversation
ReadEncoderBoundaryPoint panics with an index/slice out of range when given a truncated EBP whose flag byte advertises fields that are not actually present. The CableLabs and Comcast readers consume the extension, SAP, grouping, time and partition fields based on the flag bits without verifying the buffer still has enough bytes, so a short buffer runs the read index off the end. Bound-check each flag-driven read and return ErrInvalidEBPLength instead of panicking, and index the buffer with an int so the length math cannot wrap. Found by fuzzing ReadEncoderBoundaryPoint.
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.
ReadEncoderBoundaryPointpanics with an index/slice-out-of-range on a truncated EBP whose flag byte advertises fields that aren't actually present, so a malformed stream can crash any caller that parses EBP data.Failure mode: both
readCableLabsEbpandreadComcastEbpread the extension, SAP, grouping, time (and partition, for CableLabs) fields based purely on theDataFlagsbits, advancing an index into the buffer without checking any bytes remain. A short buffer runs the index off the end. For exampleReadEncoderBoundaryPoint([]byte{0xDF, 0x08, 0x00, 0x00, 0x00, 0x00, 0x08})sets the time flag but omits the 8 time bytes and panics withslice bounds out of range. The CableLabs grouping-extension loop has the same problem and, because the index was auint8, could also wrap.Fix: bound-check each flag-driven read and return the existing
ErrInvalidEBPLengthinstead of panicking, and index the buffer with anintso the length math can't wrap. Valid, fully-formed EBPs take the identical path and decode unchanged; existing tests still pass.Test: adds
TestReadEncoderBoundaryPointTruncatedcovering the truncated CableLabs and Comcast cases. It panics on master and passes with this change. These inputs were found by fuzzingReadEncoderBoundaryPoint.