[FFmpeg-devel] [PATCH] lavc/vvc: Validate alf_list indexes

[Frank Plowman]

On 05/02/2024 13:45, Nuo Mi wrote:
> On Sun, Feb 4, 2024 at 1:22 AM <post at frankplowman.com> wrote:
> 
>> From: Frank Plowman <post at frankplowman.com>
>>
>> Fixes crashes when decoding illegal bitstreams found by fuzzing.
>>
>> Signed-off-by: Frank Plowman <post at frankplowman.com>
>> ---
>>   libavcodec/vvc/vvc_ctu.c | 17 +++++++++++++++--
>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
>> index d166b16a19..f601c6c95e 100644
>> --- a/libavcodec/vvc/vvc_ctu.c
>> +++ b/libavcodec/vvc/vvc_ctu.c
>> @@ -2207,7 +2207,7 @@ static void hls_sao(VVCLocalContext *lc, const int
>> rx, const int ry)
>>       }
>>   }
>>
>> -static void alf_params(VVCLocalContext *lc, const int rx, const int ry)
>> +static int alf_params(VVCLocalContext *lc, const int rx, const int ry)
>>   {
>>       const VVCFrameContext *fc     = lc->fc;
>>       const H266RawSliceHeader *sh  = lc->sc->sh.r;
>> @@ -2233,6 +2233,11 @@ static void alf_params(VVCLocalContext *lc, const
>> int rx, const int ry)
>>                   c_idx == CB ? sh->sh_alf_cb_enabled_flag :
>> sh->sh_alf_cr_enabled_flag;
>>               if (alf_enabled_flag) {
>>                   const VVCALF *aps =
>> fc->ps.alf_list[sh->sh_alf_aps_id_chroma];
>> +                if (!aps) {
>> +                    // Slice header refers to an APS which does not exist
>> +                    return AVERROR_INVALIDDATA;
>> +                }
>> +
>>
> Thank you Frank.
> Since all "if conditions" are related to slices, could we perform the check
> at sh_derive()?

Yes, I had another look at this yesterday and came to the same 
conclusion.  Do you think it's better to derive pointers to the relevant 
APSs or just to validate the indices into fps->aps_list?

> 
>>                   alf->ctb_flag[c_idx] = ff_vvc_alf_ctb_flag(lc, rx, ry,
>> c_idx);
>>                   alf->alf_ctb_filter_alt_idx[c_idx - 1] = 0;
>>                   if (alf->ctb_flag[c_idx] && aps->num_chroma_filters > 1)
>> @@ -2247,10 +2252,16 @@ static void alf_params(VVCLocalContext *lc, const
>> int rx, const int ry)
>>               alf->ctb_cc_idc[i] = 0;
>>               if (cc_enabled[i]) {
>>                   const VVCALF *aps = fc->ps.alf_list[cc_aps_id[i]];
>> +                if (!aps) {
>> +                    // Slice header refers to an APS which does not exist
>> +                    return AVERROR_INVALIDDATA;
>> +                }
>>                   alf->ctb_cc_idc[i] = ff_vvc_alf_ctb_cc_idc(lc, rx, ry, i,
>> aps->num_cc_filters[i]);
>>               }
>>           }
>>       }
>> +
>> +    return 0;
>>   }
>>
>>   static void deblock_params(VVCLocalContext *lc, const int rx, const int
>> ry)
>> @@ -2274,7 +2285,9 @@ static int hls_coding_tree_unit(VVCLocalContext *lc,
>>       memset(lc->parse.chroma_qp_offset, 0,
>> sizeof(lc->parse.chroma_qp_offset));
>>
>>       hls_sao(lc, x0 >> sps->ctb_log2_size_y, y0 >> sps->ctb_log2_size_y);
>> -    alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
>> sps->ctb_log2_size_y);
>> +    ret = alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
>> sps->ctb_log2_size_y);
>> +    if (ret < 0)
>> +        return ret;
>>       deblock_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
>> sps->ctb_log2_size_y);
>>
>>       if (IS_I(rsh) && sps->r->sps_qtbtt_dual_tree_intra_flag)
>> --
>> 2.43.0
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".