From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id A39154BA09 for ; Tue, 9 Jul 2024 14:47:25 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 43A7468DCE9; Tue, 9 Jul 2024 17:47:21 +0300 (EEST) Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com [209.85.208.49]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A965668DCE3 for ; Tue, 9 Jul 2024 17:47:14 +0300 (EEST) Received: by mail-ed1-f49.google.com with SMTP id 4fb4d7f45d1cf-58ba3e38028so6619388a12.0 for ; Tue, 09 Jul 2024 07:47:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720536434; x=1721141234; darn=ffmpeg.org; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=Ds7/1Ie0q5TzzQzSV2WPZ7WdjmAI5BvPSUZQIX7d9T0=; b=Qm+Y2B7ZeOVsrrF+ODvfCDHH4Q2ZVZyaBq/x1zwB4sLY9zAVg2HEt5xopcbovRIWIv LYwSAl04OtfE7HrpIXRsbyV0j2Hi/gIa9AhVX7Cs0ULohBXVQ5HO8r/rKqd+y5Rbc3zZ loRdNMHhS+Hfkh/wMli4jqInYt2FrIDI3nDLkTXanXNmyGCxiofRcRI4pczgAg1+nXEP p5zfJ5W3EIU+7TEZtitkON/BnpVvNWxXk7uQSHWoyLLBl+dkumaIRsppLs+MnZ+x140Z HWZ8O1K6l/l+q0xgceZfsZ0d9AP3CROsAQUDYABDYE5/TNWCQRsfeXXsJFf/x3WeqUtf sQGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720536434; x=1721141234; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Ds7/1Ie0q5TzzQzSV2WPZ7WdjmAI5BvPSUZQIX7d9T0=; b=CVVjeit/AvWkIqQW4oUSs+lpqTbbuq70W+gZUcuRSTwgJ469nmLAkZFJx03eZYG16b OioksM4d4v8ntmZd4dDCddPfy7YXCTgEaz3SZeA7TVetivvTKN1ceuCIgY3L8bhUFCh2 hqUbGalwu/p6mG94bkVyPHb+kFnlh+WAZGRwO864XxPBgg53X3YcUbO5d4QAlrucYCYV h2jciyoPd5qsgMHq4al9DKCzpwOfrXh9k/ruIAguVzYB0zWPLGd4TTgWtYtXugERFtSH jgBXstMnrSJd5teQZ90txjUocDqbFVUKVgnDMPigevaO7F6v2DZo28gSxT4h75IUn84U G+XA== X-Gm-Message-State: AOJu0Yz4BJDamR6RcnO/wh6jCOjn5zOTgbIH98gwFOJ6qOWcyj6gORrq NbV5u4SuzMZmJfilCr5iMo5KdfEP7LVkwGibIm8H7t+IF8LbEtZx2Agy4B+GAegaClj/VWmASUf fXwE06A431mfIMvuCeWJlyjEhlYjtp9twKxg= X-Google-Smtp-Source: AGHT+IEb4Ndl5psIlWw8vkAW2HibEKcpouSkYlCOx5imEsq1sT6ShF2hq2t5Ru9xBHtOpvgmIcy2A6x0E3YPNfkLpQk= X-Received: by 2002:a05:6402:50d1:b0:57c:6d9a:9149 with SMTP id 4fb4d7f45d1cf-594b9ee143dmr2146607a12.15.1720536433430; Tue, 09 Jul 2024 07:47:13 -0700 (PDT) MIME-Version: 1.0 References: <20240709113711.1836747-1-michael@niedermayer.cc> <172053104893.21847.7749073807798840029@lain.khirnov.net> In-Reply-To: <172053104893.21847.7749073807798840029@lain.khirnov.net> From: Kacper Michajlow Date: Tue, 9 Jul 2024 16:46:59 +0200 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vf_scale: Cleanup some checks X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Tue, 9 Jul 2024 at 15:17, Anton Khirnov 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 > > --- > > 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-request@ffmpeg.org with subject "unsubscribe".