fix: bound num_ref_frames_in_pic_order_cnt_cycle in AVC SPS parsing - #550
Open
ChrisJr404 wants to merge 1 commit into
Open
fix: bound num_ref_frames_in_pic_order_cnt_cycle in AVC SPS parsing#550ChrisJr404 wants to merge 1 commit into
ChrisJr404 wants to merge 1 commit into
Conversation
An out-of-range num_ref_frames_in_pic_order_cnt_cycle was used directly to size a slice, so a tiny malformed SPS NAL unit could make ParseSPSNALUnit allocate multiple gigabytes. Reject values outside the spec range 0-255 (ISO/IEC 14496-10 7.4.2.1.1) before allocating.
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.
While fuzzing the un-fuzzed codec bitstream parsers, I found that
avc.ParseSPSNALUnitusesnum_ref_frames_in_pic_order_cnt_cyclestraight from the bitstream to size a slice, so a tiny malformed SPS NAL unit can force a multi-gigabyte allocation.Failure mode: when
pic_order_cnt_type == 1, the parser readsnum_ref_frames_in_pic_order_cnt_cycleas an Exp-Golomb value and immediately doesmake([]uint, numRefFramesInPicOrderCntCycle). The field is unbounded, so a 22-byte input encoding a ~1e9 value makes ParseSPSNALUnit allocate ~9 GB (measured) before the read loop hits EOF — an out-of-memory / denial-of-service on attacker-controlled input.Fix: the spec (ISO/IEC 14496-10 Section 7.4.2.1.1) constrains
num_ref_frames_in_pic_order_cnt_cycleto the range 0-255. Reject values above that viareader.SetErrorand return before allocating, matching the existing bound-check style used inhevc/sps.go. Valid streams decode byte-identically.Test: added
TestSPSParserNumRefFramesInPicOrderCntCycle, which feeds the fuzzer-found NAL unit and asserts an error is returned. Before the fix the test hangs the process in a multi-GB allocation (20s timeout); after the fix it returns an error in microseconds. Existing tests still pass.