Skip to content

bound exp-golomb suffix read to bitstream in avc_nalu_read_uev#3334

Open
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:avc-uev-bounds
Open

bound exp-golomb suffix read to bitstream in avc_nalu_read_uev#3334
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:avc-uev-bounds

Conversation

@sahvx655-wq

Copy link
Copy Markdown

avc_nalu_read_uev decodes an Exp-Golomb ue(v) in two passes. The first loop counts leading-zero bits and correctly stops on stream->empty(), but the second loop then reads leadingZeroBits suffix bits back to back with no such check, and BitStream::read_bit() dereferences its cursor unconditionally (its own comment notes empty() must be checked first). A sequence parameter set whose bitstream ends inside or just after the leading-zero run therefore makes the suffix loop walk off the end of the rbsp buffer. I traced it with a guard-page harness: handing the function a one-byte all-zero bitstream empties the stream in the first loop, leadingZeroBits comes out as 7, and the suffix loop first read_bit() lands on the following PROT_NONE page and takes a SIGBUS. The same bytes reach here from the network via AVCDecoderConfigurationRecord::Create then ParseSPS when an AVC sequence header is parsed.

The fix re-checks stream->empty() inside the suffix loop and returns -1 when the data runs out, matching what the leading-zero loop and the sibling avc_nalu_read_bit already do and what every caller already treats as a decode failure. Keeping the bound in the reader covers every Exp-Golomb field in the SPS rather than asking ParseSPS to pre-measure each one. Left as is this is an out-of-bounds read of attacker-controlled length (up to ~30 bits past the buffer) driven by a publisher-supplied sequence header, leaking adjacent memory into the decoded value or crashing the parser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant