Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:128661 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by lists.php.net (Postfix) with ESMTPS id 0546D1A00BC for ; Mon, 8 Sep 2025 21:14:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1757365994; bh=Z5NsvG8kQ+hCN+NtTjumsOE3DubXs2jChhgHxNPVR+o=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=KAXQXJrBPDmZmZ5oHPKmQSRaFA4FqhyY0PCXopKtZaqGxjZV4Y9eKtWgWBsUliCVN zOj3asD1l/RbQALrKUpb2+hf+7Fyxhls0bVHBrxZJgCcU9NduKPfkx0AYE5EalaQ3r slgqmz0UTzxTfXk3FA5vHrcHzqQs8ov47cR5jV0zYb6jKhWppxYXaDRHJIhr7JRyGd j+q40pZ7p8VPS8pLcpB+dLOL1zIh/AIEfp34sEBGf8lXDaXiwY+9pyybXb/FzTcMCC O9RYNJv/Wzjy/GpmKj/1e5tWlzfhS/HTzjyfJcTV+v2BGjjkF2tbw9i7La2FtNju/v ou0RJMbuLjw7A== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 262C31804C7 for ; Mon, 8 Sep 2025 21:13:11 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-0.9 required=5.0 tests=BAYES_00,DMARC_NONE, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, HTML_MESSAGE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=4.0.1 X-Spam-Virus: No X-Envelope-From: Received: from mail-ot1-f45.google.com (mail-ot1-f45.google.com [209.85.210.45]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Mon, 8 Sep 2025 21:13:09 +0000 (UTC) Received: by mail-ot1-f45.google.com with SMTP id 46e09a7af769-74382025891so4123131a34.3 for ; Mon, 08 Sep 2025 14:14:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757366076; x=1757970876; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=uLk3OSPaUSJMKLdmvfp1U4gK11BOvFBU9WUHqNjZdaM=; b=jdbaTm5lDsoVyIEiiV1aBgxDkn71x6EFCQu2jWQaIVizTXveuXuL05yIIVE4gjT1jr p8O5goMx0q7BhH7RGaY2Ix/zNBCZrO/0zmQhedsWrpWMCsVd2vryaUMrY3tXESctIc6/ pqKI0ub8NvgLgERZRkmWtWKs0lMndkYEFS3s8kXPavGx2br5eTJ4KAuH7stnY65kAmSU RejkhW2Lver1nmeWFVEjIKP/K+8KNnhMZTByhmDuA+LaNodxeuyMr9p5hqKfV1cQVL9A NQ0hoUnvuy/t4tkOHwAX5zaXXKmrZe+1oUdkeV0/PXNpiFx5b08nVej839cIs+8C7BvM 4vdg== X-Forwarded-Encrypted: i=1; AJvYcCVD8gQAD+F9LPTCSAGQRFlSqLe4TbDU+uGGMdQAvsPeN3BMhisTEv1zWqV8GYP2QO+7nv3J6lcgeO0=@lists.php.net X-Gm-Message-State: AOJu0Yz4gWfH/Lw8Tvohd3RG09k07U/d9DbiFgqvd6h86fEGkEKjaOBL jO5+8xy25bTXM/0v/pq8dwaqZeL0ectAGVBGpcT5MC4GfM6LtRKLSuj7p+bzt/8oQrPGwSJfAnX ms0iFodbD7ZxHRkY46/Jm0CAVN2pOUazuBkGd X-Gm-Gg: ASbGncuKcfLWEflPQP8XVOg9mMEKqoZk6WRjhLc9kbovVdqFh4/T/koIFwKPD7fTxM8 ys5sTzdeqLiwrTdX9yCB/CgkSSnqIZcB09Ir5HLLclKM9vqZRX+LcwQ+d2oYRHV7zHuEyuOkiN6 QgrVxZcmewFkBqpyxOZXCIZ+3+/+nXiDGNDFNkuMgVvTcer8yghqHzAy7an0Yp32Kmnk5s3OC4l eMfESxTBGVw2MQUAg== X-Google-Smtp-Source: AGHT+IH3TlMELxKtZwFHNDkRPO2j6fg9Vqg5gHYNAo0Ph3uzMksYA4aB08ZJVwkjwsVUTH17vJLR+CI9FI4zNH+dUMM= X-Received: by 2002:a05:6808:6c88:b0:437:decf:ab53 with SMTP id 5614622812f47-43b29ad3d9bmr4699173b6e.30.1757366075978; Mon, 08 Sep 2025 14:14:35 -0700 (PDT) Precedence: list list-help: list-post: List-Id: x-ms-reactions: disallow MIME-Version: 1.0 References: In-Reply-To: Date: Mon, 8 Sep 2025 23:14:24 +0200 X-Gm-Features: AS18NWAhLY6yOZ7CWPaE965AFsDvXczIrH7xpyZz4HoZYWdsYOHxACK2e97RIHA Message-ID: Subject: Re: [PHP-DEV] [RFC] Soft-Deprecate __sleep() and __wakeup() To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= Cc: Nicolas Grekas , PHP Internals List Content-Type: multipart/alternative; boundary="000000000000b2f770063e50ac38" From: bukka@php.net (Jakub Zelenka) --000000000000b2f770063e50ac38 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, On Mon, Sep 8, 2025 at 10:44=E2=80=AFPM Tim D=C3=BCsterhus wrote: > Hi > > On 9/8/25 17:32, Jakub Zelenka wrote: > > I think the point here was that it was close and the RFC itself was > > Again, a 2/3 majority is *not* close. Twice the number of folks were in > favor than against the RFC. > > I meant close to the actual threshold (not close vote). Again if single person changed their mind because of the facts below (which is quite likely), it might not have passed. This RFC is just about to see if people can see that there is significantly more effort needed for this migration. It just tries to make sure that people can vote on the right description. If they don't think, that's wort it to change the result, then so be it. But I think it's fair to provide an RFC with a correct description so it can be properly decided. > > misleading and omitted some important points that would like change the > > final result. > > I concede that =E2=80=9Cstraight up improvement=E2=80=9D might not be per= fectly accurate > for `__wakeup()`, but for `__sleep()` it *is* accurate. `__sleep()` is > broken and there's a straight-forward migration to `__serialize()`. > > I appreciate that both of you communicated your concerns with the > proposed deprecation during the discussion, but you basically just said > =E2=80=9CI believe the impact is too large=E2=80=9D, which is not actiona= ble feedback > for the RFC author and also doesn't help voters make an educated > decision. The point of the discussion phase is figuring out these > details together. > > Yeah as I mentioned it was also my fault not to point this out sooner but it's hard to verify every single thing in detail if there are like 50 deprecations proposed. > >> The serialization mechanism is also a security sensitive part of the > >> language, the fewer moving parts there are, the better. Security is pa= rt > >> of the motivation for me. > >> > >> > > Could you be more specific here? We do not consider issues (crashes and > > similar) resulting from unserializing of the serialized string as > security > > issues because it must not come from the untrusted source (see > > https://www.php.net/manual/en/function.unserialize.php ). I don't > remember > > any security issue in serialize / unserialize since this rule was set. > > Even when only dealing with trusted data, unserialize needs to parse > inputs (which is already sufficiently dangerous in C) and it also needs > to deal with partial(ly initialized) object graphs all while executing > arbitrary (userland) code that may or may not interact in weird ways > (e.g. when error handlers get involved due to a bug in a custom > unserialize hook). And of course there can also be bugs with the > serializer that result in =E2=80=9Ctrusted=E2=80=9D output that triggers = unexpected and > untested code-paths, particularly when references or cycles get > involved. When running with mixed PHP versions during an incremental > upgrade it might also be possible for serialization data to be emitted > by newer PHP versions that may not be anticipated by older PHP versions > and that trigger unexpected code-paths. > > I understand your concern about the complexity but this can apply to many other parts in php-src. As I'm sure you know, this still wouldn't likely to be considered as a security issue. > And then there's also stuff like > https://wiki.php.net/rfc/unserialize_warn_on_trailing_data, which can > happen due to bugs in a userland implementation even when not doing > unserialize($_GET['stuff']). > > This could be closer but we wouldn't like find exploit either. At least this wasn't even raised as a security issue so I guess you were expecting that not to be a security problem. > Saying that =E2=80=9Cunserialize is not security-relevant because you mus= t only > feed it safe inputs=E2=80=9D as an excuse to avoid making unserialize saf= er is > not helping anyone and is downplaying the risks involved in > unserialization. > > I don't have problem with making unserialize safer as there might be users that use it improperly. But we shouldn't be saying that this is a security sensitive part when we don't consider those sort of issues as security issues because none of those issues will get fixed in our security support only releases. In that sense I don't think we can consider it as a security sensitive part. Kind regards Jakub --000000000000b2f770063e50ac38 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,

On Mon, Sep 8, 2025 at 10:44= =E2=80=AFPM Tim D=C3=BCsterhus <tim@= bastelstu.be> wrote:
Hi

On 9/8/25 17:32, Jakub Zelenka wrote:
> I think the point here was that it was close and the RFC itself was
Again, a 2/3 majority is *not* close. Twice the number of folks were in favor than against the RFC.


I meant close to the actual threshold = (not close vote). Again if single person changed their mind because of the = facts below (which is quite likely), it might not have passed.
This RFC is just about to see if people can see that there is = significantly more effort needed for this migration. It just tries to make = sure that people can vote on the right description. If they don't think= , that's wort it to change the result, then so be it. But I think it= 9;s fair to provide an RFC with a correct description so it can be properly= decided.
=C2=A0
> misleading and omitted some important points that would like change th= e
> final result.

I concede that =E2=80=9Cstraight up improvement=E2=80=9D might not be perfe= ctly accurate
for `__wakeup()`, but for `__sleep()` it *is* accurate. `__sleep()` is
broken and there's a straight-forward migration to `__serialize()`.

I appreciate that both of you communicated your concerns with the
proposed deprecation during the discussion, but you basically just said =E2=80=9CI believe the impact is too large=E2=80=9D, which is not actionabl= e feedback
for the RFC author and also doesn't help voters make an educated
decision. The point of the discussion phase is figuring out these
details together.


Yeah as I mentioned it was also my fau= lt not to point this out sooner but it's hard to verify every single th= ing in detail if there are like 50 deprecations proposed.
=C2=A0<= /div>
>> The serialization mechanism is also a security sensitive part of t= he
>> language, the fewer moving parts there are, the better. Security i= s part
>> of the motivation for me.
>>
>>
> Could you be more specific here? We do not consider issues (crashes an= d
> similar) resulting from unserializing of the serialized string as secu= rity
> issues because it must not come from the untrusted source (see
> https://www.php.net/manual/en/function.un= serialize.php ). I don't remember
> any security issue in serialize / unserialize since this rule was set.=

Even when only dealing with trusted data, unserialize needs to parse
inputs (which is already sufficiently dangerous in C) and it also needs to deal with partial(ly initialized) object graphs all while executing
arbitrary (userland) code that may or may not interact in weird ways
(e.g. when error handlers get involved due to a bug in a custom
unserialize hook). And of course there can also be bugs with the
serializer that result in =E2=80=9Ctrusted=E2=80=9D output that triggers un= expected and
untested code-paths, particularly when references or cycles get
involved. When running with mixed PHP versions during an incremental
upgrade it might also be possible for serialization data to be emitted
by newer PHP versions that may not be anticipated by older PHP versions and that trigger unexpected code-paths.


I understand your concern about the co= mplexity but this can apply to many other parts in php-src. As I'm sure= you know, this still wouldn't likely to be considered as a security is= sue.
=C2=A0
https://wiki.php.net/rfc/unserialize_warn= _on_trailing_data, which can
happen due to bugs in a userland implementation even when not doing
unserialize($_GET['stuff']).


This could be closer but we wouldn'= ;t like find exploit either. At least this wasn't even raised as a secu= rity issue so I guess you were expecting that not to be a security problem.=
=C2=A0
Saying that =E2=80=9Cunserialize is not security-relevant because you must = only
feed it safe inputs=E2=80=9D as an excuse to avoid making unserialize safer= is
not helping anyone and is downplaying the risks involved in unserialization= .


I don't have problem with making u= nserialize safer as there might be users that use it improperly.=C2=A0 But = we shouldn't be saying that this is a security sensitive part when we d= on't consider those sort of issues as security issues because none of t= hose issues will get fixed in our security support only releases. In that s= ense I don't think we can consider it as a security sensitive part.

Kind regards

Jakub


--000000000000b2f770063e50ac38--