[FFmpeg-devel] [PATCH v2] lavc/vvc: Validate explicit subpic locations

[Nuo Mi]

On Mon, Aug 26, 2024 at 1:50 AM Frank Plowman <post at frankplowman.com> wrote:

> Implement the missing requirements from H.266 (V3) p. 106 on the
> position and size of subpictures whose dimensions are provided
> explicitly.
>
Applied this and "[FFmpeg-devel] [PATCH] lavc/vvc: Prevent OOB access in
subpic_tiles"
Thank you, Frank.

>
> Signed-off-by: Frank Plowman <post at frankplowman.com>
> ---
> Changes since v1 (20240824092827.68912-1-post at frankplowman.com):
> * Use temporary variables and AV_CEIL_RSHIFT to make bound calculations
>   more readable.
> * Fix bounds on size.  The calculated values are minimums, not
>   maximums as in v1.  Additionally, fix an integer overflow in the bound
>   calculations.
>
>  libavcodec/cbs_h266_syntax_template.c | 49 +++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/libavcodec/cbs_h266_syntax_template.c
> b/libavcodec/cbs_h266_syntax_template.c
> index 9c37996947..a8f5af04d0 100644
> --- a/libavcodec/cbs_h266_syntax_template.c
> +++ b/libavcodec/cbs_h266_syntax_template.c
> @@ -1061,7 +1061,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
> RWContext *rw,
>      unsigned int ctb_log2_size_y, min_cb_log2_size_y,
>                   min_qt_log2_size_intra_y, min_qt_log2_size_inter_y,
>                   ctb_size_y, max_num_merge_cand, tmp_width_val,
> tmp_height_val;
> -    uint8_t qp_bd_offset;
> +    uint8_t qp_bd_offset, sub_width_c, sub_height_c;
>
>      static const uint8_t h266_sub_width_c[] = {
>          1, 2, 2, 1
> @@ -1089,6 +1089,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
> RWContext *rw,
>
>      u(3, sps_max_sublayers_minus1, 0, VVC_MAX_SUBLAYERS - 1);
>      u(2, sps_chroma_format_idc, 0, 3);
> +    sub_width_c = h266_sub_width_c[current->sps_chroma_format_idc];
> +    sub_height_c = h266_sub_height_c[current->sps_chroma_format_idc];
> +
>      u(2, sps_log2_ctu_size_minus5, 0, 3);
>      ctb_log2_size_y = current->sps_log2_ctu_size_minus5 + 5;
>      ctb_size_y = 1 << ctb_log2_size_y;
> @@ -1110,8 +1113,6 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
> RWContext *rw,
>
>      flag(sps_conformance_window_flag);
>      if (current->sps_conformance_window_flag) {
> -        uint8_t sub_width_c =
> h266_sub_width_c[current->sps_chroma_format_idc];
> -        uint8_t sub_height_c =
> h266_sub_height_c[current->sps_chroma_format_idc];
>          uint16_t width = current->sps_pic_width_max_in_luma_samples /
> sub_width_c;
>          uint16_t height = current->sps_pic_height_max_in_luma_samples /
> sub_height_c;
>          ue(sps_conf_win_left_offset, 0, width);
> @@ -1160,19 +1161,37 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
> RWContext *rw,
>              }
>              for (i = 1; i <= current->sps_num_subpics_minus1; i++) {
>                  if (!current->sps_subpic_same_size_flag) {
> -                    if (current->sps_pic_width_max_in_luma_samples >
> ctb_size_y)
> -                        ubs(wlen, sps_subpic_ctu_top_left_x[i], 1, i);
> -                    else
> +                    if (current->sps_pic_width_max_in_luma_samples >
> ctb_size_y) {
> +                        const unsigned int win_right_edge =
> +                            current->sps_pic_width_max_in_luma_samples
> +                          - current->sps_conf_win_right_offset *
> sub_width_c;
> +                        us(wlen, sps_subpic_ctu_top_left_x[i], 0,
> +                           AV_CEIL_RSHIFT(win_right_edge,
> ctb_log2_size_y) - 1,
> +                           1, i);
> +                    } else
>                          infer(sps_subpic_ctu_top_left_x[i], 0);
>                      if (current->sps_pic_height_max_in_luma_samples >
> -                        ctb_size_y)
> -                        ubs(hlen, sps_subpic_ctu_top_left_y[i], 1, i);
> -                    else
> +                        ctb_size_y) {
> +                        const unsigned int win_bottom_edge =
> +                            current->sps_pic_height_max_in_luma_samples
> +                          - current->sps_conf_win_bottom_offset *
> sub_height_c;
> +                        us(hlen, sps_subpic_ctu_top_left_y[i], 0,
> +                           AV_CEIL_RSHIFT(win_bottom_edge,
> ctb_log2_size_y) - 1,
> +                           1, i);
> +                    } else
>                          infer(sps_subpic_ctu_top_left_y[i], 0);
>                      if (i < current->sps_num_subpics_minus1 &&
>                          current->sps_pic_width_max_in_luma_samples >
>                          ctb_size_y) {
> -                        ubs(wlen, sps_subpic_width_minus1[i], 1, i);
> +                        const unsigned int win_left_edge =
> +                            current->sps_conf_win_left_offset *
> sub_width_c;
> +                        const unsigned int win_left_edge_ctus =
> +                            AV_CEIL_RSHIFT(win_left_edge,
> ctb_log2_size_y);
> +                        us(wlen, sps_subpic_width_minus1[i],
> +                           win_left_edge_ctus >
> current->sps_subpic_ctu_top_left_x[i]
> +                               ? win_left_edge_ctus -
> current->sps_subpic_ctu_top_left_x[i]
> +                               : 0,
> +                           MAX_UINT_BITS(wlen), 1, i);
>                      } else {
>                          infer(sps_subpic_width_minus1[i],
>                                tmp_width_val -
> @@ -1181,7 +1200,15 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
> RWContext *rw,
>                      if (i < current->sps_num_subpics_minus1 &&
>                          current->sps_pic_height_max_in_luma_samples >
>                          ctb_size_y) {
> -                        ubs(hlen, sps_subpic_height_minus1[i], 1, i);
> +                        const unsigned int win_top_edge =
> +                            current->sps_conf_win_top_offset *
> sub_height_c;
> +                        const unsigned int win_top_edge_ctus =
> +                            AV_CEIL_RSHIFT(win_top_edge, ctb_log2_size_y);
> +                        us(hlen, sps_subpic_height_minus1[i],
> +                           win_top_edge_ctus >
> current->sps_subpic_ctu_top_left_y[i]
> +                               ? win_top_edge_ctus -
> current->sps_subpic_ctu_top_left_y[i]
> +                               : 0,
> +                           MAX_UINT_BITS(wlen), 1, i);
>                      } else {
>                          infer(sps_subpic_height_minus1[i],
>                                tmp_height_val -
> --
> 2.46.0
>
>