Skip to content

Commit

Permalink
mvcdec: Heap overflow in 'ih264d_parse_fgc'
Browse files Browse the repository at this point in the history
Although the fag end of both the NALU and the bitstream buffer
 is being parsed, not all FGC SEI symbols would have been
decoded semantically. This commit detects and returns an error
in this situation.

Bug = ossfuzz:65418
Test: mvc_dec_fuzzer
  • Loading branch information
AshwinNatesan-ittiam committed Jan 17, 2024
1 parent 28727ba commit d3d49af
Showing 1 changed file with 18 additions and 0 deletions.
18 changes: 18 additions & 0 deletions decoder/ih264d_sei.c
Original file line number Diff line number Diff line change
Expand Up @@ -853,12 +853,30 @@ WORD32 ih264d_parse_fgc(dec_bit_stream_t *ps_bitstrm, dec_struct_t *ps_dec,

for(i = 0; i <= ps_sei->s_sei_fgc_params.au1_num_intensity_intervals_minus1[c]; i++)
{
/* Although the fag end of both the NALU and the bitstream buffer */
/* is being parsed, not all FGC SEI symbols would have been */
/* decoded semantically. The code below detects this condition */
if((ps_bitstrm->u4_ofst + 8 + 8) >= ps_bitstrm->u4_max_ofst)
{
return ERROR_INV_SEI_FGC_PARAMS;
}

ps_sei->s_sei_fgc_params.au1_intensity_interval_lower_bound[c][i] =
(UWORD8) ih264d_get_bits_h264(ps_bitstrm, 8);

ps_sei->s_sei_fgc_params.au1_intensity_interval_upper_bound[c][i] =
(UWORD8) ih264d_get_bits_h264(ps_bitstrm, 8);

/* Although the fag end of both the NALU and the bitstream buffer */

This comment has been minimized.

Copy link
@harishdm

harishdm Jan 17, 2024

Member

each one is not 32 bits. so this is wrong.
please move this inside the loop and check for 32 bits in each iteration.

these two are different because in the second approach we are expecting 32 bits are remaining for the next iteration and if the SEV consumes let us say 2 bits in one iteration, we will only look for 32 for next iteration. In the first approach we would have looked at 64 bits.

also please run compliance tests once this change is done to ensure nothing is broken

/* is being parsed, not all FGC SEI symbols would have been */
/* decoded semantically. The code below detects this condition */
if((ps_bitstrm->u4_ofst +
ps_sei->s_sei_fgc_params.au1_num_model_values_minus1[c] * 32) >=
ps_bitstrm->u4_max_ofst)
{
return ERROR_INV_SEI_FGC_PARAMS;
}

for(j = 0; j <= ps_sei->s_sei_fgc_params.au1_num_model_values_minus1[c]; j++)
{
ps_sei->s_sei_fgc_params.ai4_comp_model_value[c][i][j] =
Expand Down

0 comments on commit d3d49af

Please sign in to comment.