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 488494261C for ; Wed, 21 Sep 2022 16:16:32 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2C59068BABE; Wed, 21 Sep 2022 19:16:30 +0300 (EEST) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0EBDE68B900 for ; Wed, 21 Sep 2022 19:16:23 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id E24966000A for ; Wed, 21 Sep 2022 16:16:22 +0000 (UTC) Date: Wed, 21 Sep 2022 18:16:21 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220921161621.GF6583@pb2> References: <20220918171410.31835-1-michael@niedermayer.cc> <20220918171410.31835-2-michael@niedermayer.cc> <5b3fa2ba445e5fae04459c5ea4f5d982d3a0a0eb.camel@acc.umu.se> <20220921093517.GD6583@pb2> <5186da17343f53dd9bd26cafe16ad4fddcc078b2.camel@acc.umu.se> MIME-Version: 1.0 In-Reply-To: <5186da17343f53dd9bd26cafe16ad4fddcc078b2.camel@acc.umu.se> Subject: Re: [FFmpeg-devel] [PATCH 02/13] avformat/mxfdec: Check run_in to fit in int and be valid 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="===============8158409870352957467==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============8158409870352957467== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="KJY2Ze80yH5MUxol" Content-Disposition: inline --KJY2Ze80yH5MUxol Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 21, 2022 at 03:23:21PM +0200, Tomas H=E4rdin wrote: > ons 2022-09-21 klockan 11:35 +0200 skrev Michael Niedermayer: > > On Tue, Sep 20, 2022 at 01:20:00PM +0200, Tomas H=E4rdin wrote: > > > tis 2022-09-20 klockan 13:07 +0200 skrev Tomas H=E4rdin: > > > > s=F6n 2022-09-18 klockan 19:13 +0200 skrev Michael Niedermayer: > > > > > Fixes: signed integer overflow: 9223372036854775807 - - > > > > > 2146905566 > > > > > cannot be represented in type 'long' > > > > > Fixes: 50993/clusterfuzz-testcase-minimized- > > > > > ffmpeg_dem_MXF_fuzzer- > > > > > 6570996594769920 > > > > >=20 > > > > > Found-by: continuous fuzzing process=20 > > > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > > > Signed-off-by: Michael Niedermayer > > > > > --- > > > > > =A0libavformat/mxfdec.c | 6 +++++- > > > > > =A01 file changed, 5 insertions(+), 1 deletion(-) > > > > >=20 > > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > > > > index e63e803aa56..da81fea3bc1 100644 > > > > > --- a/libavformat/mxfdec.c > > > > > +++ b/libavformat/mxfdec.c > > > > > @@ -3681,6 +3681,7 @@ static int > > > > > mxf_read_header(AVFormatContext > > > > > *s) > > > > > =A0=A0=A0=A0 KLVPacket klv; > > > > > =A0=A0=A0=A0 int64_t essence_offset =3D 0; > > > > > =A0=A0=A0=A0 int ret; > > > > > +=A0=A0=A0 int64_t run_in; > > > > > =A0 > > > > > =A0=A0=A0=A0 mxf->last_forward_tell =3D INT64_MAX; > > > > > =A0 > > > > > @@ -3690,7 +3691,10 @@ static int > > > > > mxf_read_header(AVFormatContext > > > > > *s) > > > > > =A0=A0=A0=A0 } > > > > > =A0=A0=A0=A0 avio_seek(s->pb, -14, SEEK_CUR); > > > > > =A0=A0=A0=A0 mxf->fc =3D s; > > > > > -=A0=A0=A0 mxf->run_in =3D avio_tell(s->pb); > > > > > +=A0=A0=A0 run_in =3D avio_tell(s->pb); > > > > > +=A0=A0=A0 if (run_in < 0 || run_in !=3D (int)run_in) > > > >=20 > > > > run_in > INT_MAX is more clear > > > >=20 > > > > It strikes me that run_in is also used in lots of places in the > > > > demuxer > > > > without checking for overflow > > >=20 > > > I went and checked S377m and the run-in sequence "shall be less > > > than > > > 65536 bytes long". Both the 2004 and 2009 version of the spec agree > > > on > > > this. So we should reject run_in >=3D 65536, and mxf_probe() should > > > be > > > similarly adjusted. > >=20 > > ok, will do > >=20 > > thx for checking > >=20 > > i will change the patch by: > > @@ -3717,7 +3717,7 @@ static int mxf_read_header(AVFormatContext *s) > > =A0=A0=A0=A0 avio_seek(s->pb, -14, SEEK_CUR); > > =A0=A0=A0=A0 mxf->fc =3D s; > > =A0=A0=A0=A0 run_in =3D avio_tell(s->pb); > > -=A0=A0=A0 if (run_in < 0 || run_in !=3D (int)run_in) > > +=A0=A0=A0 if (run_in < 0 || run_in > 65535) >=20 > Let's avoid magic numbers: > #define RUN_IN_MAX 65535 // S377m-2004 section 5.5 and S377-1-2009 > section 6.5 sure, will use this thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB it is not once nor twice but times without number that the same ideas make their appearance in the world. -- Aristotle --KJY2Ze80yH5MUxol Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYys40QAKCRBhHseHBAsP qzumAJ4xsfUvK7IXiHPJd11XLHCtpKOViwCfW7cok1jR8FuejTnzUhy4lIfFezk= =ncfH -----END PGP SIGNATURE----- --KJY2Ze80yH5MUxol-- --===============8158409870352957467== 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". --===============8158409870352957467==--