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 BE2F840BB6 for ; Sat, 7 May 2022 18:57:52 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 50CF068B3A1; Sat, 7 May 2022 21:57:50 +0300 (EEST) Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id DBE3E68B092 for ; Sat, 7 May 2022 21:57:43 +0300 (EEST) Received: from localhost (213-47-68-29.cable.dynamic.surfer.at [213.47.68.29]) (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 0A1771BF208 for ; Sat, 7 May 2022 18:57:42 +0000 (UTC) Date: Sat, 7 May 2022 20:57:41 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220507185741.GJ396728@pb2> References: MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH v2 02/11] libavformat/asfdec: fix get_value return type and add checks for 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: multipart/mixed; boundary="===============7526770871436549751==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============7526770871436549751== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="vkEkAx9hr54EJ73W" Content-Disposition: inline --vkEkAx9hr54EJ73W Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, May 07, 2022 at 09:36:35AM +0000, softworkz wrote: > From: softworkz >=20 > unsupported values >=20 > get_value had a return type of int, which means that reading > QWORDS (case 4) was broken due to truncation of the result from > avio_rl64(). >=20 > Signed-off-by: softworkz > --- > libavformat/asfdec_f.c | 38 +++++++++++++++++++++++++++++--------- > 1 file changed, 29 insertions(+), 9 deletions(-) >=20 > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c > index a8f36ed286..d31e1d581d 100644 > --- a/libavformat/asfdec_f.c > +++ b/libavformat/asfdec_f.c > @@ -202,7 +202,7 @@ static int asf_probe(const AVProbeData *pd) > =20 > /* size of type 2 (BOOL) is 32bit for "Extended Content Description Obje= ct" > * but 16 bit for "Metadata Object" and "Metadata Library Object" */ > -static int get_value(AVIOContext *pb, int type, int type2_size) > +static uint64_t get_value(AVIOContext *pb, int type, int type2_size) > { > switch (type) { > case ASF_BOOL: > @@ -567,10 +567,22 @@ static int asf_read_ext_content_desc(AVFormatContex= t *s, int64_t size) > /* My sample has that stream set to 0 maybe that mean the contai= ner. > * ASF stream count starts at 1. I am using 0 to the container v= alue > * since it's unused. */ > - if (!strcmp(name, "AspectRatioX")) > - asf->dar[0].num =3D get_value(s->pb, value_type, 32); > - else if (!strcmp(name, "AspectRatioY")) > - asf->dar[0].den =3D get_value(s->pb, value_type, 32); > + if (!strcmp(name, "AspectRatioX")) { > + const uint64_t value =3D get_value(s->pb, value_type, 32); > + if (value > INT32_MAX) { > + av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioX value:= %"PRIu64"\n", value); > + return AVERROR(ENOTSUP); > + } > + asf->dar[0].num =3D (int)value; > + } > + else if (!strcmp(name, "AspectRatioY")) { > + const uint64_t value =3D get_value(s->pb, value_type, 32); > + if (value > INT32_MAX) { > + av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioY value:= %"PRIu64"\n", value); > + return AVERROR(ENOTSUP); > + } > + asf->dar[0].den =3D (int)value; > + } > else > get_tag(s, name, value_type, value_len, 32); > } > @@ -630,13 +642,21 @@ static int asf_read_metadata(AVFormatContext *s, in= t64_t size) > i, stream_num, name_len_utf16, value_type, value_len, na= me); > =20 > if (!strcmp(name, "AspectRatioX")){ > - int aspect_x =3D get_value(s->pb, value_type, 16); > + const uint64_t aspect_x =3D get_value(s->pb, value_type, 16); > + if (aspect_x > INT32_MAX) { > + av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioX value:= %"PRIu64"\n", aspect_x); > + return AVERROR(ENOTSUP); > + } > if(stream_num < 128) > - asf->dar[stream_num].num =3D aspect_x; > + asf->dar[stream_num].num =3D (int)aspect_x; > } else if(!strcmp(name, "AspectRatioY")){ > - int aspect_y =3D get_value(s->pb, value_type, 16); > + const uint64_t aspect_y =3D get_value(s->pb, value_type, 16); > + if (aspect_y > INT32_MAX) { > + av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioY value:= %"PRIu64"\n", aspect_y); > + return AVERROR(ENOTSUP); > + } > if(stream_num < 128) > - asf->dar[stream_num].den =3D aspect_y; > + asf->dar[stream_num].den =3D (int)aspect_y; > } else { If you go to the length to do something with oddly huge aspect components maybe change dar to 2 uint64_t and check it in one place instead of 2 also the av_reduce() can handle a wider range than int32 thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 2 "100% positive feedback" - "All either got their money back or didnt compla= in" "Best seller ever, very honest" - "Seller refunded buyer after failed scam" --vkEkAx9hr54EJ73W Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYnbBJQAKCRBhHseHBAsP qz7IAJ9OZtpL84bZDCQGw3mjiTgTysxOqwCffO2UubVFXJcv6CWFUEq6PpWXAf8= =1eiS -----END PGP SIGNATURE----- --vkEkAx9hr54EJ73W-- --===============7526770871436549751== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============7526770871436549751==--