On Sun, Jul 20, 2025 at 1:17 AM Frank Plowman <p...@frankplowman.com> wrote:
> Prior to this patch, kth_order_egk_decode could read arbitrarily
> large values which then overflowed and caused various issues.
> Patch fixes this by making kth_order_egk_decode falliable,
> requiring the caller to specify an upper bound and returning an
> error if the read value would exceed that bound.
>
> This patch resolves the same issue as
> eb52251c0ab025b6b40b28994bc9dc616813b190, but I think this is the proper
> fix as it also addresses issues with syntax elements besides
> ff_vvc_num_signalled_palette_entries.
>
+1, thank you, frank
>
> Signed-off-by: Frank Plowman <p...@frankplowman.com>
> ---
> Changes since v2:
>
> Allow the pattern
>
> ret = falliable_func(args);
> // Do something with ret
> if (ret < 0)
> return ret;
>
> instead of only
>
> ret = falliable_func(args);
> if (ret < 0)
> return ret;
>
> // Do something with ret
>
> In order to reduce number of lines touched by this patch.
> ---
> libavcodec/vvc/cabac.c | 19 ++++++++++++-------
> libavcodec/vvc/cabac.h | 6 +++---
> libavcodec/vvc/ctu.c | 19 ++++++++++---------
> 3 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/libavcodec/vvc/cabac.c b/libavcodec/vvc/cabac.c
> index 6847ce59af..c2dbd46709 100644
> --- a/libavcodec/vvc/cabac.c
> +++ b/libavcodec/vvc/cabac.c
> @@ -929,7 +929,7 @@ static int truncated_binary_decode(VVCLocalContext
> *lc, const int c_max)
> }
>
> // 9.3.3.5 k-th order Exp - Golomb binarization process
> -static int kth_order_egk_decode(CABACContext *c, int k)
> +static int kth_order_egk_decode(CABACContext *c, int k, const int max)
> {
> int bit = 1;
> int value = 0;
> @@ -937,6 +937,8 @@ static int kth_order_egk_decode(CABACContext *c, int k)
>
> while (bit) {
> bit = get_cabac_bypass(c);
> + if (max - value < (bit << k))
> + return AVERROR_INVALIDDATA;
> value += bit << k++;
> }
>
> @@ -946,6 +948,9 @@ static int kth_order_egk_decode(CABACContext *c, int k)
> value += symbol;
> }
>
> + if (value > max)
> + return AVERROR_INVALIDDATA;
> +
> return value;
> }
>
> @@ -1377,14 +1382,14 @@ int ff_vvc_intra_chroma_pred_mode(VVCLocalContext
> *lc)
> return (get_cabac_bypass(&lc->ep->cc) << 1) |
> get_cabac_bypass(&lc->ep->cc);
> }
>
> -int ff_vvc_palette_predictor_run(VVCLocalContext *lc)
> +int ff_vvc_palette_predictor_run(VVCLocalContext *lc, const int max)
> {
> - return kth_order_egk_decode(&lc->ep->cc, 0);
> + return kth_order_egk_decode(&lc->ep->cc, 0, max);
> }
>
> -int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc)
> +int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc, const int
> max)
> {
> - return kth_order_egk_decode(&lc->ep->cc, 0);
> + return kth_order_egk_decode(&lc->ep->cc, 0, max);
> }
>
> int ff_vvc_new_palette_entries(VVCLocalContext *lc, const int bit_depth)
> @@ -1424,9 +1429,9 @@ int ff_vvc_palette_idx_idc(VVCLocalContext *lc,
> const int max_palette_index, con
> return truncated_binary_decode(lc, max_palette_index - adjust);
> }
>
> -int ff_vvc_palette_escape_val(VVCLocalContext *lc)
> +int ff_vvc_palette_escape_val(VVCLocalContext *lc, const int max)
> {
> - return kth_order_egk_decode(&lc->ep->cc, 5);
> + return kth_order_egk_decode(&lc->ep->cc, 5, max);
> }
>
> int ff_vvc_general_merge_flag(VVCLocalContext *lc)
> diff --git a/libavcodec/vvc/cabac.h b/libavcodec/vvc/cabac.h
> index 972890317e..6a0e713d19 100644
> --- a/libavcodec/vvc/cabac.h
> +++ b/libavcodec/vvc/cabac.h
> @@ -81,15 +81,15 @@ int ff_vvc_intra_luma_mpm_remainder(VVCLocalContext
> *lc);
> int ff_vvc_cclm_mode_flag(VVCLocalContext *lc);
> int ff_vvc_cclm_mode_idx(VVCLocalContext *lc);
> int ff_vvc_intra_chroma_pred_mode(VVCLocalContext *lc);
> -int ff_vvc_palette_predictor_run(VVCLocalContext *lc);
> -int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc);
> +int ff_vvc_palette_predictor_run(VVCLocalContext *lc, const int max);
> +int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc, const int
> max);
> int ff_vvc_new_palette_entries(VVCLocalContext *lc, int bit_dpeth);
> bool ff_vvc_palette_escape_val_present_flag(VVCLocalContext *lc);
> bool ff_vvc_palette_transpose_flag(VVCLocalContext *lc);
> bool ff_vvc_run_copy_flag(VVCLocalContext *lc, int prev_run_type, int
> prev_run_position, int cur_pos);
> bool ff_vvc_copy_above_palette_indices_flag(VVCLocalContext *lc);
> int ff_vvc_palette_idx_idc(VVCLocalContext *lc, int max_palette_index,
> bool adjust);
> -int ff_vvc_palette_escape_val(VVCLocalContext *lc);
> +int ff_vvc_palette_escape_val(VVCLocalContext *lc, const int max);
>
> //inter
> int ff_vvc_general_merge_flag(VVCLocalContext *lc);
> diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c
> index 35c18e78f6..d54e6a322b 100644
> --- a/libavcodec/vvc/ctu.c
> +++ b/libavcodec/vvc/ctu.c
> @@ -20,6 +20,7 @@
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> */
>
> +#include "libavutil/error.h"
> #include "libavutil/refstruct.h"
>
> #include "cabac.h"
> @@ -1857,16 +1858,16 @@ static int palette_predicted(VVCLocalContext *lc,
> const bool local_dual_tree, in
> }
>
> for (int i = 0; i < predictor_size && nb_predicted < max_entries;
> i++) {
> - const int run = ff_vvc_palette_predictor_run(lc);
> + const int run = ff_vvc_palette_predictor_run(lc, predictor_size -
> i);
> + if (run < 0)
> + return run;
> +
> if (run == 1)
> break;
>
> if (run > 1)
> i += run - 1;
>
> - if (i >= predictor_size)
> - return AVERROR_INVALIDDATA;
> -
> predictor_reused[i] = true;
> for (int c = start; c < end; c++)
> cu->plt[c].entries[nb_predicted] = lc->ep->pp[c].entries[i];
> @@ -1885,11 +1886,11 @@ static int palette_signaled(VVCLocalContext *lc,
> const bool local_dual_tree,
> const VVCSPS *sps = lc->fc->ps.sps;
> CodingUnit *cu = lc->cu;
> const int nb_predicted = cu->plt[start].size;
> - const int nb_signaled = nb_predicted < max_entries ?
> ff_vvc_num_signalled_palette_entries(lc) : 0;
> + const int nb_signaled = nb_predicted < max_entries ?
> ff_vvc_num_signalled_palette_entries(lc, max_entries - nb_predicted) : 0;
> const int size = nb_predicted + nb_signaled;
> const bool dual_tree_luma = local_dual_tree && cu->tree_type ==
> DUAL_TREE_LUMA;
>
> - if (size > max_entries || nb_signaled < 0)
> + if (nb_signaled < 0)
> return AVERROR_INVALIDDATA;
>
> for (int c = start; c < end; c++) {
> @@ -2052,10 +2053,10 @@ static int palette_subblock_data(VVCLocalContext
> *lc,
> if (!(xc & hs) && !(yc & vs)) {
> const int v = PALETTE_INDEX(xc, yc);
> if (v == esc) {
> - const int coeff = ff_vvc_palette_escape_val(lc);
> - if (coeff >= (1U << sps->bit_depth))
> - return AVERROR_INVALIDDATA;
> + const int coeff = ff_vvc_palette_escape_val(lc, (1 <<
> sps->bit_depth) - 1);
> const int pixel = av_clip_intp2(RSHIFT(coeff * scale,
> 6), sps->bit_depth);
> + if (coeff < 0)
> + return AVERROR_INVALIDDATA;
> PALETTE_SET_PIXEL(xc, yc, pixel);
> } else {
> PALETTE_SET_PIXEL(xc, yc, plt->entries[v]);
> --
> 2.47.0
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
