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 470864C750 for ; Tue, 6 Aug 2024 22:11:41 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9015F68DA3E; Wed, 7 Aug 2024 01:11:38 +0300 (EEST) Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B3B9A68D9D2 for ; Wed, 7 Aug 2024 01:11:31 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id DD42740002 for ; Tue, 6 Aug 2024 22:11:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1722982291; 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=KdjEn5I5pmHChi4ySXR9CFaZboYJC+C/oa2/5ZcgbGU=; b=N65OhRZzEZhpWscfTdhIzYhjBliMumM6Q1vHczMOgPl4R79MATFmlQhIsg660sBTCWLSRb mGhSobDMQAwn4oq2Icas/DgknUVzls2kiF7oxrBd8CxthW9LQWQGse5BQWt6xtj3r2QjI0 IJP6UqAS4cYZeMnisJUCkC42MhJq5mlU9p7VjyfFjqH2017PXsa9+mNOoJVyAO15JWwhl1 w6nd/XehXjFUO1ytjXLIL66+HT+BCgE07IBmTwplnhO4NQTuGVJy4zveBJY28IcBfi4ZVV aFaWu9kaoZMZ2TLiLBRJB8c6lVRmCPvkU1kQAO6Hobz4dsiDVzOuUTw3BWTfBQ== Date: Wed, 7 Aug 2024 00:11:29 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240806221129.GO4991@pb2> References: <20240806170538.GI4991@pb2> <20240806173823.GJ4991@pb2> <640d9bb8-4ed6-4cbd-ba53-2ef7a36dc558@gmail.com> <20240806191244.GN4991@pb2> 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="===============4165522957190904808==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============4165522957190904808== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Va8DSfvMR3bM4AuH" Content-Disposition: inline --Va8DSfvMR3bM4AuH Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 06, 2024 at 09:51:10PM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > 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 c= ode > >>>>> 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 = even > >>> > >>> Is this a scenario where a slice that referenced an SPS was parsed, t= hen > >>> 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? > >> > >> 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 n= ew > >> 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 parsi= ng > >> code contains a hack to make this work in the common case of a single = SPS. > >=20 > > 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 b= ut > > refer to each other is just hard to do without being exploitable. > >=20 > > 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. >=20 > This overlooks that one already needs the correct SPS for parsing. how is "Just refer to SPS for parsing" overlooking the SPS for parsing? The problem is not the parsing, the problem is the writing doesnt match the parsing. The simple fix i refered to is to store the parsed value and u= se it when writing. Not to somehow hope that a network of invalid and attacker control= led data structures, some of which might contain different values at different = times will look the same to teh writer than the parser. Thats what i meant but you can fix it anyway you want. Thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Homeopathy is like voting while filling the ballot out with transparent ink. Sometimes the outcome one wanted occurs. Rarely its worse than filling out a ballot properly. --Va8DSfvMR3bM4AuH Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZrKfjAAKCRBhHseHBAsP q5CmAJ9ivhq0b1cl7a15R6ugzzeelUqCMACfeVCjAlVqY0Kvu6wJlhLQVLK78Ns= =LXRl -----END PGP SIGNATURE----- --Va8DSfvMR3bM4AuH-- --===============4165522957190904808== 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". --===============4165522957190904808==--