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 D9B2D491FF for ; Mon, 5 Feb 2024 20:07:46 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7D91E68D145; Mon, 5 Feb 2024 22:07:45 +0200 (EET) Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6A3C568CA50 for ; Mon, 5 Feb 2024 22:07:38 +0200 (EET) Received: by mail.gandi.net (Postfix) with ESMTPSA id 8D602FF803 for ; Mon, 5 Feb 2024 20:07:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1707163657; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=snuW0nuOvdz7ljDecMsbxXnD0/1K3JrYegTx3NeHTB0=; b=PbcPbWQL/Js9QxtyKYw6zT/YdH0BRaDI81SCB6+Ukm4xDm/HSJC7ig5zZXdem0sKmUEtam 0yEK5eb5fYRBz4fl8Bu5zlk3mm71OYIHMqeTyFElTDEQGxQAeVa+DDW9iY1pW81LOK1M2K Z9kTkwsjdpUqaSepBeRS0bE5g4zwSNhwdCmImeMFLzWDuL+vPzQ7Ofu9n0OkOLyx2p36UZ MHJOh5mZkalsm2HI6G4vY0qTB0DUl4Bh03qPouIC0oUWR9Prr2/VgCL7EE/5CmRLPBnr97 +3hko33G+jhYzO4gisfO5HqbPLEc/opKs0QTo1A+QTHIRRSQXS8fK6yljtYnIw== Date: Mon, 5 Feb 2024 21:07:36 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240205200736.GS6420@pb2> References: MIME-Version: 1.0 In-Reply-To: X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH] [mov] Avoid OOM for invalid STCO / CO64 constructions. 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="===============0205436800959716156==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============0205436800959716156== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="O/aokfTXCQryhxHj" Content-Disposition: inline --O/aokfTXCQryhxHj Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Feb 02, 2024 at 03:45:24PM -0800, Dale Curtis wrote: > On Fri, Feb 2, 2024 at 3:42=E2=80=AFPM Dale Curtis wrote: >=20 > > On Fri, Feb 2, 2024 at 3:20=E2=80=AFPM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> Dale Curtis: > >> > + // Clamp allocation size for `chunk_offsets` -- don't throw an > >> error for an > >> > + // invalid count since the EOF path doesn't throw either. > >> > + entries =3D > >> > + FFMIN(entries, FFMIN(atom.size - 8, avio_size(pb) - > >> avio_tell(pb)) / > >> > + (atom.type =3D=3D MKTAG('s', 't', 'c', '= o') ? 4 > >> : 8)); > >> > + > >> > >> This may call avio_size() and avio_tell() multiple times. Furthermore, > >> is it even certain that avio_size() returns a sane value? > >> > > > > I hope so since there are other usages of avio_size() throughout the fi= le > > in a similar manner. I guess you're saying it may be invalid when > > !AVIO_SEEKABLE_NORMAL? Sticking to just atom.size is also fine. > > >=20 > Here's a version of the patch which does just that. > mov.c | 7 +++++++ > 1 file changed, 7 insertions(+) > d5ba3836202adc762f38f03cbb5e30921e6073b4 stco-clamp-entries-v2.patch > From b76f526a01788a11e625eb1d7d7005a1959df75c Mon Sep 17 00:00:00 2001 > From: Dale Curtis > Date: Fri, 2 Feb 2024 20:49:44 +0000 > Subject: [PATCH] [mov] Avoid OOM for invalid STCO / CO64 constructions. >=20 > The `entries` value is read directly from the stream and used to > allocate memory. This change clamps `entries` to however many are > possible in the remaining atom or file size (whichever is smallest). >=20 > Fixes https://crbug.com/1429357 >=20 > Signed-off-by: Dale Curtis > --- > libavformat/mov.c | 7 +++++++ > 1 file changed, 7 insertions(+) >=20 > diff --git a/libavformat/mov.c b/libavformat/mov.c > index af95e1f662..25e5beadcf 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -2237,6 +2237,13 @@ static int mov_read_stco(MOVContext *c, AVIOContex= t *pb, MOVAtom atom) > av_log(c->fc, AV_LOG_WARNING, "Ignoring duplicated STCO atom\n"); > return 0; > } > + > + // Clamp allocation size for `chunk_offsets` -- don't throw an error= for an > + // invalid count since the EOF path doesn't throw either. > + entries =3D > + FFMIN(entries, (atom.size - 8) / > + (atom.type =3D=3D MKTAG('s', 't', 'c', 'o') ?= 4 : 8)); assuming atom.size is an arbitrary 64bit value then the value of FFMIN() is also 64bit but entries is unsigned 32bit, this= truncation would allow setting entries to values outside whats expected from FFMIN() also we seem to disalllow entries =3D=3D 0 before this and its maybe possible to set entries =3D 0 here, bypassing the =3D=3D 0 ch= eck before thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras --O/aokfTXCQryhxHj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZcFABQAKCRBhHseHBAsP qxuCAKCDipi6F4UsXydhx89CPALZza1lAQCeJxVui4HCUSBGugXQqf/dT3NytgA= =obqo -----END PGP SIGNATURE----- --O/aokfTXCQryhxHj-- --===============0205436800959716156== 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". --===============0205436800959716156==--