[FFmpeg-devel] [PATCH 1/8] lavc/cbs_h265: Disallow nonsensically large HVCC NAL arrays

[Mark Thompson]

On 29/09/2019 20:54, James Almer wrote:
> On 9/29/2019 1:45 PM, Mark Thompson wrote:
>> Fixes CID 1419833.
>> ---
>>  libavcodec/cbs_h2645.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>> index 2dc261f7a5..185c458f61 100644
>> --- a/libavcodec/cbs_h2645.c
>> +++ b/libavcodec/cbs_h2645.c
>> @@ -695,7 +695,12 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx,
>>          nb_arrays = bytestream2_get_byte(&gbc);
>>          for (i = 0; i < nb_arrays; i++) {
>>              nal_unit_type = bytestream2_get_byte(&gbc) & 0x3f;
>> +
>>              nb_nals = bytestream2_get_be16(&gbc);
>> +            if (nb_nals > 64) {
> 
> Why not check for the actual limit of each ps type instead?

Mainly because that would require a big switch statement to determine the limit - since it's an early check for invalid streams which will fail later anyway I didn't want to clutter the code too much.

>                                                             This code
> will still try to parse the file if it reports more than 16 sps, for
> example, despite it being invalid.
> 
> Maybe also check for nb_nals == 0.

Is that actually invalid rather than just pointless?  I don't have a specification for this, and hevc_parse.c also accepts it (not checking the count at all).

>> +                // Too many NALs of this type - the header must be invalid.
>> +                return AVERROR_INVALIDDATA;
>> +            }
>>  
>>              start = bytestream2_tell(&gbc);
>>              for (j = 0; j < nb_nals; j++) {
>>

Thanks,

- Mark