[FFmpeg-devel] [PATCH 2/3] avcodec/cbs_h266_syntax_template: sanity check num_multi_layer_olss

[Frank Plowman]

On 29/01/2024 21:13, James Almer wrote:
> On 1/29/2024 5:19 PM, Frank Plowman wrote:
>> Below is a patch which addresses the issue, an integer overflow when 
>> calculating the bounds for
>> vps_num_ols_timing_hrd_params_minus1.  There's also a similar fix for 
>> vps_num_dpb_params_minus1.
>>
>> diff --git a/libavcodec/cbs_h266_syntax_template.c 
>> b/libavcodec/cbs_h266_syntax_template.c
>> index 549d021211..49bf2e45ac 100644
>> --- a/libavcodec/cbs_h266_syntax_template.c
>> +++ b/libavcodec/cbs_h266_syntax_template.c
>> @@ -946,7 +946,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx, 
>> RWContext *rw,
>>
>>       if (!current->vps_each_layer_is_an_ols_flag) {
>>           uint16_t vps_num_dpb_params;
>> -        ue(vps_num_dpb_params_minus1, 0, num_multi_layer_olss - 1);
>> +        ue(vps_num_dpb_params_minus1, 0,
>> +           num_multi_layer_olss > 0 ? num_multi_layer_olss - 1 : 0);
>
> FFMAX(0, num_multi_layer_olss - 1); looks better.
>
> If the spec explicitly states num_multi_layer_olss - 1 should be used 
> here, wouldn't that mean that it doesn't expect num_multi_layer_olss 
> to be 0 at this point for vps_ols_mode_idc >= 0 && vps_ols_mode_idc < 3?
> When vps_each_layer_is_an_ols_flag is true, num_multi_layer_olss is 
> inferred as 1

num_multi_layer_olss is 0 if vps_each_layer_is_an_ols_flag is true, it 
is num_layers_in_ols which must be 1.

> so I'd expect it to also be at least 1 for 
> vps_each_layer_is_an_ols_flag == false.

Yes I think you're right.  The spec isn't especially clear, but the 
description of the vps_each_layer_is_an_ols_flag leads me to believe we 
are safe to assert that.  Patch which does so below.

diff --git a/libavcodec/cbs_h266_syntax_template.c 
b/libavcodec/cbs_h266_syntax_template.c
index 549d021211..5ae3fb9f08 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -912,6 +912,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx, 
RWContext *rw,
                  num_multi_layer_olss++;
              }
          }
+        if (!current->vps_each_layer_is_an_ols_flag && 
num_multi_layer_olss == 0)
+            return AVERROR_INVALIDDATA;
      }

      for (i = 0; i <= current->vps_num_ptls_minus1; i++) {