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 EFC8349EA9 for ; Tue, 12 Mar 2024 22:13:50 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BE5F168CE3D; Wed, 13 Mar 2024 00:13:47 +0200 (EET) 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 5780C68CC87 for ; Wed, 13 Mar 2024 00:13:41 +0200 (EET) Received: by mail.gandi.net (Postfix) with ESMTPSA id DC35960002 for ; Tue, 12 Mar 2024 22:13:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1710281620; 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=MEuvA4M6pp78JO1IPakqfKjsFzOMWzjP/FxhIzEMN7U=; b=FV8oFBRoc9kjyeC9DYtUe5RryR64TPqhtMhVUxLWpAWAZ+NwqyMo0OJTC3AkgLs70+Jxqd v6+MTlSm9USH8Iy0pwfGBAaoUM+uGaBaP1nAE9BTzMFe/fU2Hj/QVdd/ir67m1MYFaL+Cx Q8PdT2rs1GViEG3UiM/KXQNusA1Zn5zeexJBnoDpCoHB9SkfnecBOQVW6kMd73zacwhCdr Yq+pzLGUcoPGMbunz78+EcSKYtxWaE7DRsqRWi5+Nt54p1sbIyZgCiuRgrSdkVz/RcXLvk WM9lfqMYVtxd3WD1CE6tF9B47vwvQvSZNVPrB90iAniD4sB8o5e7UrrLU+Uh5Q== Date: Tue, 12 Mar 2024 23:13:39 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240312221339.GW6420@pb2> References: <20230622003038.20969-1-michael@niedermayer.cc> <626B6C70-0258-444F-9AC3-6F83E539576D@gmx.de> MIME-Version: 1.0 In-Reply-To: <626B6C70-0258-444F-9AC3-6F83E539576D@gmx.de> X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH] avcodec/parser: Check next against buffer index 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="===============0721958238480724835==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============0721958238480724835== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Td38glO7oQJiYhgf" Content-Disposition: inline --Td38glO7oQJiYhgf Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jun 24, 2023 at 10:13:48PM +0200, Reimar D=F6ffinger wrote: > Hi! >=20 > > On 24 Jun 2023, at 21:14, Andreas Rheinhardt wrote: > >=20 > > Michael Niedermayer: > >> Fixes: out of array access > >> Fixes: crash-0d640731c7da52415670eb47a2af701cbe2e1a3b > >>=20 > >> Found-by: Catena cyber > >> Signed-off-by: Michael Niedermayer > >> --- > >> libavcodec/parser.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >>=20 > >> diff --git a/libavcodec/parser.c b/libavcodec/parser.c > >> index efc28b8918..db39e698ab 100644 > >> --- a/libavcodec/parser.c > >> +++ b/libavcodec/parser.c > >> @@ -214,7 +214,7 @@ int ff_combine_frame(ParseContext *pc, int next, > >> for (; pc->overread > 0; pc->overread--) > >> pc->buffer[pc->index++] =3D pc->buffer[pc->overread_index++]; > >>=20 > >> - if (next > *buf_size) > >> + if (next > *buf_size || (next < -pc->index && next !=3D END_NOT_F= OUND)) > >> return AVERROR(EINVAL); > >>=20 > >> /* flush remaining if EOF */ > >=20 > > Could you provide more details about this? E.g. which parser is this > > about at all? And how can we actually come in this situation at all? > > (Whenever I looked at ff_combine_frame() I do not really understand what > > its invariants are supposed to be.) >=20 > Yeah, when I looked at it I also felt like it has all kinds of > assumptions/preconditions without which it will break, but those > are not documented. Not really reviewable for correctness. > The change I proposed to fix the same issue was as below. But > note that it is not based on actual understanding, just that generally > index, overread_index and buf_size are updated together so I > thought it suspicious buf_size was not updated on realloc failure. > --- a/libavcodec/parser.c > +++ b/libavcodec/parser.c > @@ -252,6 +252,7 @@ int ff_combine_frame(ParseContext *pc, int next, > AV_INPUT_BUFFER_PADDING_SIZE); > if (!new_buffer) { > av_log(NULL, AV_LOG_ERROR, "Failed to reallocate parser buffe= r to %d\n", next + pc->index + AV_INPUT_BUFFER_PADDING_SIZE); > + *buf_size =3D > pc->overread_index =3D > pc->index =3D 0; > return AVERROR(ENOMEM); It appears neither of the 2 fixes was applied this is not ideal I will apply reimars patch thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No snowflake in an avalanche ever feels responsible. -- Voltaire --Td38glO7oQJiYhgf Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZfDTkAAKCRBhHseHBAsP q1J1AJ9HU8QpA3HwBa4A+mqYmF17Xq/3wQCgi7D7AWEyBFb2uh+z6HVNkXZM0MY= =r265 -----END PGP SIGNATURE----- --Td38glO7oQJiYhgf-- --===============0721958238480724835== 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". --===============0721958238480724835==--