From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
by master.gitmailbox.com (Postfix) with ESMTPS id D48F34CF66
for <ffmpegdev@gitmailbox.com>; Tue, 15 Apr 2025 18:55:49 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 27B58687D1E;
Tue, 15 Apr 2025 21:55:46 +0300 (EEST)
Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net
[217.70.183.196])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5F4F2680275
for <ffmpeg-devel@ffmpeg.org>; Tue, 15 Apr 2025 21:55:39 +0300 (EEST)
Received: by mail.gandi.net (Postfix) with ESMTPSA id AD79D433EE
for <ffmpeg-devel@ffmpeg.org>; Tue, 15 Apr 2025 18:55:38 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc;
s=gm1; t=1744743338;
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=v8RJmkHL+1E/pBzF8R31J1M1CsagQXSef9qIFxqQ4Gs=;
b=k2wO+6GEKM0gFEB7VOI7A6SmsMlXc82aXppWNIvEqWRAEF4l16w9MO7jl1dE9v3zpAY0EQ
uAbrIJsg9oo47rtFL1KQvMlCaBhFSA7Xayye0cccrqCvBEsUU/OIvJs7f26IJF3PNAGhTI
Xpz6PfvLQI/udSznmecsDdCFAVw/bOxI2Trc4qhmNOVAK9ETQXo37Tssk95dsZ+maKDjET
7cjqlzILB/wTeJCJ/klAHu9hq8+ohsPTNwOWt9u539hfI3O+DWt0PnxA3wifxDCa/9huVS
XKUbW9WUTOav5Cpncta4CEfW/OJ0vg9Qe335nc9grYDkvghXL6gZSKmo+jw8HA==
Date: Tue, 15 Apr 2025 20:55:38 +0200
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <20250415185538.GR4991@pb2>
References: <20250411222719.2779176-1-michael@niedermayer.cc>
<20250411222719.2779176-2-michael@niedermayer.cc>
<DM8P223MB0365A90C10FBC9EF6512F9E1BAB12@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
<20250414231950.GN4991@pb2>
<DM8P223MB0365928500AB53069869DC99BAB32@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
MIME-Version: 1.0
In-Reply-To: <DM8P223MB0365928500AB53069869DC99BAB32@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>
X-GND-State: clean
X-GND-Score: -90
X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvvdegvdeiucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfghrlhcuvffnffculddutddmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefoihgthhgrvghlucfpihgvuggvrhhmrgihvghruceomhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtqeenucggtffrrghtthgvrhhnpeffledtfeevfeffheeuuefhtdejieelueeftdeitdfgheetgefffeefteekffdthfenucffohhmrghinhepfhhfmhhpvghgrdhorhhgnecukfhppeeguddrieeirdeijedruddufeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepihhnvghtpeeguddrieeirdeijedruddufedphhgvlhhopehlohgtrghlhhhoshhtpdhmrghilhhfrhhomhepmhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtpdhnsggprhgtphhtthhopedupdhrtghpthhtohepfhhfmhhpvghgqdguvghvvghlsehffhhmphgvghdrohhrgh
X-GND-Sasl: michael@niedermayer.cc
Subject: Re: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that
decode_str() did advance
X-BeenThere: ffmpeg-devel@ffmpeg.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: multipart/mixed; boundary="===============3219332205673922656=="
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/20250415185538.GR4991@pb2/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
--===============3219332205673922656==
Content-Type: multipart/signed; micalg=pgp-sha512;
protocol="application/pgp-signature"; boundary="FZ/aR/5C0h9UtTSM"
Content-Disposition: inline
--FZ/aR/5C0h9UtTSM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Mon, Apr 14, 2025 at 11:59:02PM +0000, softworkz . wrote:
>=20
>=20
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Michael Niedermayer
> > Sent: Dienstag, 15. April 2025 01:20
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that
> > decode_str() did advance
> >=20
> > On Sat, Apr 12, 2025 at 01:49:53AM +0000, softworkz . wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Michael Niedermayer
> > > > Sent: Samstag, 12. April 2025 00:27
> > > > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > > > Subject: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that
> > > > decode_str() did advance
> > > >
> > > > Fixes infinite loop with unknown encodings
> > > >
> > > > We could alternatively error out from decode_str() or consume all
> > of
> > > > taglen
> > > > this would affect other callers though.
> > > >
> > > > Fixes: 409819224/clusterfuzz-testcase-minimized-
> > ffmpeg_dem_H261_fuzzer-
> > > > 6003527535362048
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > > libavformat/id3v2.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> > > > index 90314583a74..e3f7f9e2a90 100644
> > > > --- a/libavformat/id3v2.c
> > > > +++ b/libavformat/id3v2.c
> > > > @@ -341,10 +341,13 @@ static void read_ttag(AVFormatContext *s,
> > > > AVIOContext *pb, int taglen,
> > > > taglen--; /* account for encoding type byte */
> > > >
> > > > while (taglen > 1) {
> > > > + int current_taglen =3D taglen;
> > > > if (decode_str(s, pb, encoding, &dst, &taglen) < 0) {
> > > > av_log(s, AV_LOG_ERROR, "Error reading frame %s,
> > > > skipped\n", key);
> > > > return;
> > > > }
> > > > + if (current_taglen =3D=3D taglen)
> > > > + return;
> > > >
> > > > count++;
> > > >
> > > > --
> > > > 2.49.0
> > > >
> > > > _______________________________________________
> > >
> > > Hi Michael,
> > >
> > > this kind of conflicts with this patch that I had submitted
> > recently:
> > >
> > >
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.54.ffstaging.FF
> > mpeg.1740873449247.ffmpegagent@gmail.com/
> > >
> > >
> > > I wonder whether my patch would still be prone to the issue your
> > patch is addressing -
> >=20
> > This already conflicts with rcombs patch in git master, i think
> > Applying: Fixes Trac ticket https://trac.ffmpeg.org/ticket/6949
> > Using index info to reconstruct a base tree...
> > M libavformat/id3v2.c
> > Falling back to patching base and 3-way merge...
> > Auto-merging libavformat/id3v2.c
> > CONFLICT (content): Merge conflict in libavformat/id3v2.c
> > error: Failed to merge in the changes.
> > Patch failed at 0001 Fixes Trac ticket
> > https://trac.ffmpeg.org/ticket/6949
> >=20
> >=20
> > > do you have a test file perhaps?
> >=20
> > Will email you one, but the loop with a function that doesnt advance
> > is an issue even if the specific file doesnt trigger it in a different
> > implementation
> >=20
> > also probaly a good idea if you contact rcombs as you seemed to work
> > on
> > the same code
> >=20
> > I was looking at teh ticket and saw a link to rcombs patch, looked at
> > the patch and applied it. I did not realize there where 2 patches
>=20
>=20
> Hi Michael,
>=20
> I know the rcombs patch, but it has a - let's say - different behavior.
> Let's look at an example where artist and genre have multiple values:
>=20
>=20
> This was ffmpeg output unpatched:
>=20
> Metadata:
> title : Infinite (Original Mix)
> artist : B-Front
> track : 1
> album : Infinite
> date : 2017
> genre : Hardstyle
> TBPM : 150
> compilation : 0
> album_artist : B-Front
> publisher : Roughstate
>=20
>=20
> This is what the rcombs patch does:
>=20
> Metadata:
> title : Infinite (Original Mix)
> artist : B-Front
> artist : Second Artist Example
> track : 1
> album : Infinite
> date : 2017
> genre : Hardstyle
> genre : Test
> genre : Example
> genre : Hard Dance
> TBPM : 150
> compilation : 0
> album_artist : B-Front
> publisher : Roughstate
>=20
>=20
>=20
> My path does that:
>=20
> Metadata:
> title : Infinite (Original Mix)
> artist : B-Front;Second Artist Example
> track : 1
> album : Infinite
> date : 2017
> genre : Hardstyle;Test;Example;Hard Dance
> TBPM : 150
> compilation : 0
> album_artist : B-Front
> publisher : Roughstate
Iam perfectly fine with either way
but i have to point out that the 2nd method has some problems too
for example checking if foo is an author becomes more difficult
theres also a question about scalability if there are many entries
And what exactly do you do if an Artist or Title itself contains a ;
Thx
[...]
--=20
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
--FZ/aR/5C0h9UtTSM
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZ/6rpgAKCRBhHseHBAsP
q9xWAJ9qaaAuCF0StIpU7TyXxv3+89hapgCggXDD8iz3Dwskh2GeL2edFblwGqQ=
=6IVm
-----END PGP SIGNATURE-----
--FZ/aR/5C0h9UtTSM--
--===============3219332205673922656==
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".
--===============3219332205673922656==--