On Tue, 9 Jul 2024 at 15:17, Anton Khirnov <an...@khirnov.net> wrote: > > Quoting Michael Niedermayer (2024-07-09 13:37:11) > > Fixes: CID1513722 Operands don't affect result > > > > Sponsored-by: Sovereign Tech Fund > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > --- > > libavfilter/vf_scale.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c > > index bf09196e10d..18e9393d6c1 100644 > > --- a/libavfilter/vf_scale.c > > +++ b/libavfilter/vf_scale.c > > @@ -645,10 +645,8 @@ static int config_props(AVFilterLink *outlink) > > if (ret < 0) > > goto fail; > > > > - if (outlink->w > INT_MAX || > > - outlink->h > INT_MAX || > > - (outlink->h * inlink->w) > INT_MAX || > > - (outlink->w * inlink->h) > INT_MAX) > > + if ((outlink->h * (int64_t)inlink->w) > INT32_MAX || > > + (outlink->w * (int64_t)inlink->h) > INT32_MAX) > > This does not seems cleaner to me. > > Also, this check overall seems fishy. Why is it here at all and not e.g. > in ff_scale_adjust_dimensions()? Why does it not call > av_image_check_size()? Why does it only print a warning and not do > anything else?
I agree with Anton here. The checks in vf_scale are iffy at best. For starters, this is `outlink->w > INT_MAX` dead check. As the `w` is int already. And there is already an UB in `scale_eval_dimensions()` which converts double value to int without any checks. I think something like this would make sense to reject big numbers, and ofcourse ff_scale_adjust_dimensions() should be more clever about overflows too. There is also an overflow in swscale, but that's another story. diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index a1a322ed9e..9483db7564 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -537,7 +537,7 @@ static int scale_eval_dimensions(AVFilterContext *ctx) const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format); const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(outlink->format); char *expr; - int eval_w, eval_h; + double eval_w, eval_h; int ret; double res; const AVPixFmtDescriptor *main_desc; @@ -588,23 +588,22 @@ static int scale_eval_dimensions(AVFilterContext *ctx) } res = av_expr_eval(scale->w_pexpr, scale->var_values, NULL); - eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res; + eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = res == 0 ? inlink->w : res; res = av_expr_eval(scale->h_pexpr, scale->var_values, NULL); - if (isnan(res)) { - expr = scale->h_expr; + eval_h = scale->var_values[VAR_OUT_H] = scale->var_values[VAR_OH] = res == 0 ? inlink->h : res; + + if (isnan(eval_w) || eval_w < INT_MIN || eval_w > INT_MAX) { + expr = scale->w_expr; ret = AVERROR(EINVAL); goto fail; } - eval_h = scale->var_values[VAR_OUT_H] = scale->var_values[VAR_OH] = (int) res == 0 ? inlink->h : (int) res; - res = av_expr_eval(scale->w_pexpr, scale->var_values, NULL); - if (isnan(res)) { - expr = scale->w_expr; + if (isnan(eval_h) || eval_h < INT_MIN || eval_h > INT_MAX) { + expr = scale->h_expr; ret = AVERROR(EINVAL); goto fail; } - eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res; scale->w = eval_w; scale->h = eval_h; _______________________________________________ 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".