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 2BDE04C82A for ; Tue, 6 Aug 2024 19:12:55 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C6FA068DA7F; Tue, 6 Aug 2024 22:12:52 +0300 (EEST) Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2797368DA0D for ; Tue, 6 Aug 2024 22:12:46 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 6BD461C0004 for ; Tue, 6 Aug 2024 19:12:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1722971565; 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=jXnNqdBqnGFUl/TSw7AFhgs2JhLzrx4Tu49nWGaSTjE=; b=RsQovgVef0vy9ReAmFePFvkN4KxT+X7wMLiZXRiq0Y3a+skBrFjq9pm2YHPTfLO6QNWxlb nLJyUMfXS01Gcih0SAdM0bES+7WB0qblKdTD0ULv+TlXXgJJq1j7tC/9S6YZgY3a57dMl2 4b5bXHTEAwNusGqlVZaFGeJAxpbVIedxRwz81fkyLmZ5rBbBI05pRJBOuUf0XGLF5kjghk zyc7SK0E/tX83ln4UgvhGtuAk4E7xvvtejxE3Kos+XdAsOasjc/xKHqY+02hoLpLBP6mzu HFo9e/UvJcfZewUYtrHz1Cz//4NusqiUHIt+cMV+7jBk3kXjnpzU074MSWk/4Q== Date: Tue, 6 Aug 2024 21:12:44 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240806191244.GN4991@pb2> References: <20240806170538.GI4991@pb2> <20240806173823.GJ4991@pb2> <640d9bb8-4ed6-4cbd-ba53-2ef7a36dc558@gmail.com> MIME-Version: 1.0 In-Reply-To: X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] CBS 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="===============4395535058924378212==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============4395535058924378212== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="V3ENGWVrnMOBWwz8" Content-Disposition: inline --V3ENGWVrnMOBWwz8 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 06, 2024 at 08:41:16PM +0200, Andreas Rheinhardt wrote: > James Almer: > > On 8/6/2024 2:54 PM, Andreas Rheinhardt wrote: > >> Michael Niedermayer: > >>> On Tue, Aug 06, 2024 at 07:05:38PM +0200, Michael Niedermayer wrote: > >>>> Hi > >>>> > >>>> Did CBS win the obfuscated C contest yet? > >>>> > >>>> I was just looking at a msan issue and then looked at this: > >>>> > >>>> CHECK(FUNC_SEI(message_list)(ctx, rw, ¤t->message_list, 1)); > >>>> > >>>> > >>>> #define CHECK(call) do { \ > >>>> =A0=A0=A0=A0=A0=A0=A0=A0 err =3D (call); \ > >>>> =A0=A0=A0=A0=A0=A0=A0=A0 if (err < 0) \ > >>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return err; \ > >>>> =A0=A0=A0=A0 } while (0) > >>>> > >>>> #define FUNC_NAME2(rw, codec, name) cbs_ ## codec ## _ ## rw ## _ ## > >>>> name > >>>> #define FUNC_NAME1(rw, codec, name) FUNC_NAME2(rw, codec, name) > >>>> #define FUNC_H264(name) FUNC_NAME1(READWRITE, h264, name) > >>>> #define FUNC_H265(name) FUNC_NAME1(READWRITE, h265, name) > >>>> #define FUNC_H266(name) FUNC_NAME1(READWRITE, h266, name) > >>>> #define FUNC_SEI(name)=A0 FUNC_NAME1(READWRITE, sei,=A0 name) > >>>> > >>>> #define SEI_FUNC(name, args) \ > >>>> static int FUNC(name) args;=A0 \ > >>>> static int FUNC(name ## _internal)(CodedBitstreamContext *ctx, \ > >>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 RWContext *rw, void *cur,=A0=A0 \ > >>>> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 SEIMessageState *state)=A0=A0=A0=A0= \ > >>>> { \ > >>>> =A0=A0=A0=A0 return FUNC(name)(ctx, rw, cur, state); \ > >>>> } \ > >>>> static int FUNC(name) args > >>>> > >>>> > >>>> anyway, can we remove all preprocessor use from cbs ? > >> > >> I don't think that this is really obfuscated. > >> > >>> > >>> the issue iam looking at is due to > >>> > >>> SEI_FUNC(sei_pic_timing, (CodedBitstreamContext *ctx, RWContext *rw, > >>> H264RawSEIPicTiming *current, SEIMessageState *sei)) > >>> > >>> having different active SPS on writing than reading, so the write code > >>> has nal_hrd_parameters_present_flag set while the read had that 0 > >>> so uninitialized data is written > >>> > >>> I cannot find any match for "cbs" in MAINTAINERS, also there are no > >>> copyright > >>> with names in the cbs code. > >> > >> 1. I just sent a patch that "fixes" this. > >> 2. But actually, there is a deeper bug here: We would need to defer > >> parsing certain SEI message units to a second pass when the currently > >> active SPS is known. This can happen with spec-compliant input (and ev= en > >=20 > > Is this a scenario where a slice that referenced an SPS was parsed, then > > a SEI message, then another slice that references another SPS, and the > > SEI expects the latter to be active despite it being coded before the > > slice? >=20 > Your example is not spec-compliant (at least not if the input is > supposed to only contain a single access unit). It can happen if there > is an SEI, then new parameter sets and then a slice activating these new > parameter sets. Or it can happen if there are multiple parameter sets > and an SEI followed by a slice activating a different SPS. The current > code is also wrong when parsing the first packet: Because in this case > no SPS has been activated yet, film_grain and sei_pic_timing SEI parsing > code contains a hack to make this work in the common case of a single SPS. Having an attacker be able to supply SPS, SPS activation and SPS use in an atacker choosen order. While these elements are not self contained but refer to each other is just hard to do without being exploitable. One way to fix this is to make the sei self contained and not refer to the = SPS at all. Just refer to SPS for parsing but then when storing just store the = fields that where parsed not depending on the right SPS being (still) there. thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Any man who breaks a law that conscience tells him is unjust and willingly= =20 accepts the penalty by staying in jail in order to arouse the conscience of= =20 the community on the injustice of the law is at that moment expressing the= =20 very highest respect for law. - Martin Luther King Jr --V3ENGWVrnMOBWwz8 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZrJ1qQAKCRBhHseHBAsP q28MAKCAErMuoZYMx7fgrRBbQ/HuP1XY/wCeLFCRWM2oiGJniTg6dQQBv6Qjdjc= =pjwz -----END PGP SIGNATURE----- --V3ENGWVrnMOBWwz8-- --===============4395535058924378212== 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". --===============4395535058924378212==--