[FFmpeg-devel] [PATCH] avcodec/cbs_h266: fix SPS VUI extension data leak

[Andreas Rheinhardt]

Nuo Mi:
> On Mon, Oct 23, 2023 at 7:46 PM Andreas Rheinhardt <
> andreas.rheinhardt at outlook.com> wrote:
> 
>> Nuo Mi:
>>> On Sun, Oct 22, 2023 at 8:58 PM Andreas Rheinhardt <
>>> andreas.rheinhardt at outlook.com> wrote:
>>>
>>>> Nuo Mi:
>>>>> Fixes: VUI extension leak
>>>>> Fixes:
>>>>
>> 63004/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-4928832253329408
>>>>>
>>>>> Found-by: continuous fuzzing process
>>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>>>> ---
>>>>>  libavcodec/cbs_h2645.c | 10 +++++++++-
>>>>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>>>>> index 470f60b95f..ef631a11fe 100644
>>>>> --- a/libavcodec/cbs_h2645.c
>>>>> +++ b/libavcodec/cbs_h2645.c
>>>>> @@ -1979,6 +1979,13 @@ static const CodedBitstreamUnitTypeDescriptor
>>>> cbs_h265_unit_types[] = {
>>>>>      CBS_UNIT_TYPE_END_OF_LIST
>>>>>  };
>>>>>
>>>>> +static void cbs_h266_free_sps(FFRefStructOpaque unused, void *content)
>>>>> +{
>>>>> +    H266RawSPS *sps = (H266RawSPS*)content;
>>>>> +    av_buffer_unref(&sps->extension_data.data_ref);
>>>>> +    av_buffer_unref(&sps->vui.extension_data.data_ref);
>>>>> +}
>>>>> +
>>>>>  static void cbs_h266_free_sei(FFRefStructOpaque unused, void *content)
>>>>>  {
>>>>>      H266RawSEI *sei = content;
>>>>> @@ -1989,7 +1996,6 @@ static const CodedBitstreamUnitTypeDescriptor
>>>> cbs_h266_unit_types[] = {
>>>>>      CBS_UNIT_TYPE_INTERNAL_REF(VVC_DCI_NUT, H266RawDCI,
>>>> extension_data.data),
>>>>>      CBS_UNIT_TYPE_INTERNAL_REF(VVC_OPI_NUT, H266RawOPI,
>>>> extension_data.data),
>>>>>      CBS_UNIT_TYPE_INTERNAL_REF(VVC_VPS_NUT, H266RawVPS,
>>>> extension_data.data),
>>>>> -    CBS_UNIT_TYPE_INTERNAL_REF(VVC_SPS_NUT, H266RawSPS,
>>>> extension_data.data),
>>>>>      CBS_UNIT_TYPE_INTERNAL_REF(VVC_PPS_NUT, H266RawPPS,
>>>> extension_data.data),
>>>>>      CBS_UNIT_TYPE_INTERNAL_REF(VVC_PREFIX_APS_NUT, H266RawAPS,
>>>> extension_data.data),
>>>>>      CBS_UNIT_TYPE_INTERNAL_REF(VVC_SUFFIX_APS_NUT, H266RawAPS,
>>>> extension_data.data),
>>>>> @@ -2003,6 +2009,8 @@ static const CodedBitstreamUnitTypeDescriptor
>>>> cbs_h266_unit_types[] = {
>>>>>      CBS_UNIT_RANGE_INTERNAL_REF(VVC_IDR_W_RADL, VVC_GDR_NUT,
>>>>>                                  H266RawSlice, data),
>>>>>
>>>>> +    CBS_UNIT_TYPE_COMPLEX(VVC_SPS_NUT, H266RawSPS, cbs_h266_free_sps),
>>>>> +
>>>>>      CBS_UNIT_TYPES_COMPLEX((VVC_PREFIX_SEI_NUT, VVC_SUFFIX_SEI_NUT),
>>>>>                             H266RawSEI, cbs_h266_free_sei),
>>>>>
>>>>
>>>> This should be a CBS_UNIT_TYPE_INTERNAL_REF().
>>>>
>>> Hi Andreas,
>>> Thank you for the review.
>>>
>>> Do you mean the SEI?
>>>
>>
>> No, I mean the SPS. (If you want to, I can send a patch for this leak.)
>>
> Sure, please go ahead.
> 
> Thank you
> 

Sent a patch. Rereading this I came to realize that my advice was not
helpful, given that VVC_SPS_NUT already uses CBS_UNIT_TYPE_INTERNAL_REF.
The solution is to use CBS_CONTENT_TYPE_INTERNAL_REFS, but with two
internal refs. There is no macro for this atm (and the only other unit
with two internal refs uses CBS_UNIT_TYPE_RANGE, so one can't simply use
a common macro for them).

- Andreas