Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:128664 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 01C761A00BC for ; Wed, 10 Sep 2025 09:26:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1757496316; bh=Z7ZeLaanmFsVLQmntZzIi9XFjerB0VNG02SdVPx8jxc=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=O1hsQWzSlboYHK3HNcapYwkqh0TlUa1zzNwTwrnRDJhBNPRD36qS+0BoETYEBqRYB N8QXnuLaeNoarVLNrBiIw/JqV1aa+BrJdkKCnaCodwEkWxjfVT5RFdKdseP4SS/YRV tI81WwTUbbaAC6fA8bxx4HIVg5by5vyqsvhgg3uruVFHqeG25QYMU2C4tD8oXzcEzl NRqpSa39lAtDBv3fxMbIV2ySKjV7sZEYcKxrVT+B5rPew5vIglz8gCgkEvvc3x1Ze4 9BWVxjrlQWAwsJ5f1/n/JgQsPbu7vlBmtI6Hzmjk+T7pKpX87BV1R5eKgkkguKayQt zm7GrnlJyAAcw== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id BEAF118006A for ; Wed, 10 Sep 2025 09:25:15 +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.5 required=5.0 tests=BAYES_05,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-oa1-f46.google.com (mail-oa1-f46.google.com [209.85.160.46]) (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 ; Wed, 10 Sep 2025 09:25:15 +0000 (UTC) Received: by mail-oa1-f46.google.com with SMTP id 586e51a60fabf-30cce86052cso4880092fac.1 for ; Wed, 10 Sep 2025 02:26:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757496401; x=1758101201; 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=SPOQ9HfiqZEUydFPWbtmfjVv5rYonG9sn/BilXtNF1c=; b=Q2a1ee12aIqfJWN7CnU1/a7lotFWAJl60jCchqhonIwQZmp3mK0FJAebMDUS4ERiUN 3F083DRH472gZ6Zr4uriTcxuodTmf0hU15AXj/lUVmdDTyUvJ/z5GkJV3KzH8+P/nvAY ESTC/KQly4hKV3inIwyM7cvC1ay17sqzMFqmnEMmsaZPU9m9WUarYnEwYsae273R1bbd QX4ohKg0tJxz5QtSW2vTX1W7DWqrm8jrTKqqabGxv8rA6A37yVw3ynxHUc5o6ECNWgfj 2aRnBu50bQeMiSPL23mzFXAho9o0BkW7b6F4gv2RpFWSaa8wqmkMaUYIswr5zBgr+n2A cecw== X-Forwarded-Encrypted: i=1; AJvYcCWaLfsvKSkK4Pl4NnWmeHXxQ4GeK+o353WY/onksBSvGecW9WbOF4qKRo85d4Z/0etzPX0kdWgr8fQ=@lists.php.net X-Gm-Message-State: AOJu0YwH6a6P048Mybq/marG85kkjpVm1U4kpDgck+JSpMS/+tV16UnQ cT63WwUlA+9D8/JDbnV1JqJT12YZLjYXbtwLwxfSoIg/NTlZPLvQUJ+4fYM7Y4oXykdxmE0Hz5D t7BIu3P0SlaA1LjTuh59Rgli7poPWULqVyQ== X-Gm-Gg: ASbGncvlxJpFFR9J3fvQIs3D5nWnpJ6CP3LlH+rmBPNFidnWmYqz6v+xPB43ugNKiqa bqAw/P1Nk2vOVfdd87/7oB1FNZxU7i49Jykk5qBguNj3V0in/2b/onCuhayZSYrdnpX+IpH9Jma ZpgLe822l90D2g8plN/MVENaEBdmbqwKCXo1OGdYaD/7ZmbcffmgWloZvKimOBo1xbdAlrUMyKp FAx2yk= X-Google-Smtp-Source: AGHT+IFH59WrKKP2dBJzQmjlL+U8K9dAGmSdQzgpenB7dWSQ3roPofSV7KsPxmFnXdjxewMTOfR4KUAmtX5sowU10Ek= X-Received: by 2002:a05:6871:ac08:b0:314:b6a6:689d with SMTP id 586e51a60fabf-32265c2feb2mr7150542fac.49.1757496401546; Wed, 10 Sep 2025 02:26:41 -0700 (PDT) Precedence: list list-help: list-post: List-Id: x-ms-reactions: disallow MIME-Version: 1.0 References: <7b1b5b71cd32bad2646164c624dc6c7f@bastelstu.be> In-Reply-To: <7b1b5b71cd32bad2646164c624dc6c7f@bastelstu.be> Date: Wed, 10 Sep 2025 11:26:29 +0200 X-Gm-Features: Ac12FXwPHe87NodRkc7UoLaGNiqypIFt74xZn7REvUOcfnTCdAZQyxZMia4EBxw 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="000000000000b552e7063e6f0417" From: bukka@php.net (Jakub Zelenka) --000000000000b552e7063e6f0417 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, On Tue, Sep 9, 2025 at 3:39=E2=80=AFPM Tim D=C3=BCsterhus wrote: > Hi > > Am 2025-09-08 23:14, schrieb Jakub Zelenka: > > 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. > > I've intentionally said =E2=80=9Csecurity sensitive=E2=80=9D. I believe t= he risk of > security issues for the unserializer is significantly higher than in > other parts of the language and I also believe that by keeping > complexity down it is both easier to verify correctness and also to fix > any bugs that crop up. From what I see __wakeup() and __unserialize() > work quite differently internally. When there's __unserialize(), the > deserialization process only creates an empty object shell (similarly to > newInstanceWithoutConstructor) and then calls __unserialize() at the > very end. For __wakeup() all the properties are filled directly and then > __wakeup() is called at the end. In both cases the destructor is skipped > for all following objects once one of the deserialization hooks fails. > > For __wakeup() this has the interesting effect that in case of circular > structures, some objects may appear to already be properly initialized > (all the properties are there), but __wakeup() has not executed yet, > potentially making them unsafe to touch. In case of __unserialize() the > objects are clearly in a partially initialized state (e.g. properties > being uninit), which I'd claim is safer: > > > class A { > public $a; > > public function __construct(public string $name) > { > > } > > public function __unserialize(array $data): void > { > $this->a =3D $data['a']; > $this->name =3D $data['name']; > echo "Waking up ", $this->name, PHP_EOL; > var_dump($this->a->name); > } > > public function __wakeup(): void > { > echo "Waking up ", $this->name, PHP_EOL; > var_dump($this->a->name); > } > > public function __destruct() > { > echo __METHOD__, PHP_EOL; > } > } > > $a =3D new A('A'); > $a->a =3D new A('B'); > $a->a->a =3D new A('C'); > $a->a->a->a =3D $a; > > echo "Before", PHP_EOL; > var_dump(unserialize(serialize($a))); > echo "After", PHP_EOL; > > In case of `__unserialize()` the unsafe access to `$this->a->name` will > throw, whereas in `__wakeup()` it will return `A`, despite `A` not being > fully available yet. Similarly skipping the destructor for an empty > object shell is safer than skipping the destructor for an object that > may appear usable. > > >> 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 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. > > The unserializer already contains quite a bit of logic to make it as > safe as possible, e.g. running the unserialization hooks only at the > very end and skipping destructors for objects where the unserialization > hook didn't successfully run. The forefathers definitely considered > possible edge cases that might lead to security issues. > > I can see what you mean but you could say in the same way that many parts in the engine and other parts are security sensitive. Almost anything that is complex can lead to a security issue given some preconditions. The fact is that we have not had any serialization issue for a long time so to me there are places that are more security sensitive and more problematic than seriallization. Kind regards, Jakub --000000000000b552e7063e6f0417 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi,

On Tue, Sep 9, 2025 = at 3:39=E2=80=AFPM Tim D=C3=BCsterhus <tim@bastelstu.be> wrote:
Hi

Am 2025-09-08 23:14, schrieb Jakub Zelenka:
> I understand your concern about the complexity but this can apply to <= br> > many
> other parts in php-src. As I'm sure you know, this still wouldn= 9;t
> likely to
> be considered as a security issue.

I've intentionally said =E2=80=9Csecurity sensitive=E2=80=9D. I believe= the risk of
security issues for the unserializer is significantly higher than in
other parts of the language and I also believe that by keeping
complexity down it is both easier to verify correctness and also to fix any bugs that crop up. From what I see __wakeup() and __unserialize()
work quite differently internally. When there's __unserialize(), the deserialization process only creates an empty object shell (similarly to newInstanceWithoutConstructor) and then calls __unserialize() at the
very end. For __wakeup() all the properties are filled directly and then __wakeup() is called at the end. In both cases the destructor is skipped for all following objects once one of the deserialization hooks fails.

For __wakeup() this has the interesting effect that in case of circular structures, some objects may appear to already be properly initialized
(all the properties are there), but __wakeup() has not executed yet,
potentially making them unsafe to touch. In case of __unserialize() the objects are clearly in a partially initialized state (e.g. properties
being uninit), which I'd claim is safer:

=C2=A0 =C2=A0 =C2=A0<?php

=C2=A0 =C2=A0 =C2=A0class A {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public $a;

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public function __construct(public string= $name)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public function __unserialize(array $data= ): void
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0$this->a =3D $data['= a'];
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0$this->name =3D $data[&#= 39;name'];
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "Waking up "= , $this->name, PHP_EOL;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0var_dump($this->a->na= me);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public function __wakeup(): void
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "Waking up "= , $this->name, PHP_EOL;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0var_dump($this->a->na= me);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public function __destruct()
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo __METHOD__, PHP_EOL; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0 =C2=A0}

=C2=A0 =C2=A0 =C2=A0$a =3D new A('A');
=C2=A0 =C2=A0 =C2=A0$a->a =3D new A('B');
=C2=A0 =C2=A0 =C2=A0$a->a->a =3D new A('C');
=C2=A0 =C2=A0 =C2=A0$a->a->a->a =3D $a;

=C2=A0 =C2=A0 =C2=A0echo "Before", PHP_EOL;
=C2=A0 =C2=A0 =C2=A0var_dump(unserialize(serialize($a)));
=C2=A0 =C2=A0 =C2=A0echo "After", PHP_EOL;

In case of `__unserialize()` the unsafe access to `$this->a->name` wi= ll
throw, whereas in `__wakeup()` it will return `A`, despite `A` not being fully available yet. Similarly skipping the destructor for an empty
object shell is safer than skipping the destructor for an object that
may appear usable.

>> 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 unserial= ize safer 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.=C2=A0 But we shouldn't be saying that this= is a
> security
> sensitive part when we don't consider those sort of issues as secu= rity
> 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.

The unserializer already contains quite a bit of logic to make it as
safe as possible, e.g. running the unserialization hooks only at the
very end and skipping destructors for objects where the unserialization hook didn't successfully run. The forefathers definitely considered possible edge cases that might lead to security issues.


I can see what you mean but you could say in the same = way that many parts in the engine and other parts are security sensitive. A= lmost anything that is complex can lead to a security issue given some prec= onditions. The fact is that we have not had any serialization issue for a l= ong time so to me there are places that are more security sensitive and mor= e problematic than seriallization.

Kind regards,

Jakub
--000000000000b552e7063e6f0417--