Fix GH-24674: Share the unserialize context only with Serializable

NEWS

@@ -152,6 +152,8 @@ PHP                                                                        NEWS
     lookup, and an unchecked CreateFileA() failure. (Ilia Alshanetsky)
   . Fix persistent stream context lifetime during shutdown (Levi Morrison)
   . Fixed bug GH-23780 (unserialize('') fails silently). (timwolla)
+  . Fixed bug GH-24674 (unserialize() called while another value is being
+    unserialized reads that value's back-references). (Ilia Alshanetsky)
   . Fixed stream_select() aborting the whole call once any watched stream's
     descriptor exceeded FD_SETSIZE; it no longer imposes that limit, so watching
     many descriptors no longer requires rebuilding with --enable-fd-setsize.

UPGRADING

@@ -55,6 +55,17 @@ PHP 8.7 UPGRADE NOTES
   . The number of filters that can be chained in a php://filter URL is limited
     to 16 by default. Set the stream context option max_filter_count to change
     this.
+  . unserialize() called from a destructor, an error handler or another
+    callback that runs while a value is being unserialized no longer resolves
+    R: and r: back-references into the outer value. Only the
+    Serializable::unserialize() call made for a C: payload, and code it calls,
+    shares them.
+  . A C: payload unserialized from __wakeup() or __unserialize() now honours
+    the allowed_classes and max_depth options of that unserialize() call, and
+    unserialize() calls made while it is processed inherit them.
+  . Starting, suspending or resuming a Fiber inside the
+    Serializable::unserialize() call made by unserialize() for a C: payload
+    now throws a FiberError.
 
 ========================================
 2. New Features

UPGRADING.INTERNALS

@@ -77,6 +77,14 @@ PHP 8.7 INTERNALS UPGRADE NOTES
 - Standard:
   . php_explode() now takes a HashTable* instead of a zval* to store the
     resulting parts. It already expected the zval* to be IS_ARRAY.
+  . BG(unserialize).level has been removed. BG(unserialize).data is now set
+    only while unserialize() calls the unserialize handler of a C: payload,
+    and is NULL otherwise; code that writes it must save and restore it. The
+    first PHP_VAR_UNSERIALIZE_INIT() made while it is set shares that
+    context until the matching PHP_VAR_UNSERIALIZE_DESTROY() if
+    BG(serialize_lock) is zero, as php_var_serialize_init() does. Otherwise
+    it creates its own context, which takes allowed_classes, max_depth and
+    incomplete_classes_only from the shared one.
 
 ========================
 4. OpCode changes

ext/session/tests/gh24674.phpt

@@ -0,0 +1,32 @@
+--TEST--
+GH-24674 (Destructor run by session_decode() gets its own unserialize() context)
+--EXTENSIONS--
+session
+--INI--
+session.use_cookies=0
+session.cache_limiter=
+session.serialize_handler=php
+session.save_handler=files
+--FILE--
+<?php
+class D {
+    public function __destruct() {
+        echo "destructor: ";
+        var_dump(@unserialize('R:1;'));
+    }
+}
+
+session_start();
+$_SESSION['a'] = new D;
+session_decode('b|s:6:"secret";a|i:5;');
+var_dump($_SESSION);
+session_abort();
+?>
+--EXPECT--
+destructor: bool(false)
+array(2) {
+  ["a"]=>
+  int(5)
+  ["b"]=>
+  string(6) "secret"
+}

ext/standard/basic_functions.h

@@ -88,7 +88,6 @@ typedef struct _php_basic_globals {
 	} serialize;
 	struct {
 		struct php_unserialize_data *data;
-		unsigned level;
 	} unserialize;
 
 	/* url_scanner_ex.re */

ext/standard/tests/serialize/gh24674.phpt

@@ -0,0 +1,67 @@
+--TEST--
+GH-24674 (Nested unserialize() shares the context of a parse in progress)
+--FILE--
+<?php
+class D {
+    public function __destruct() {
+        echo "destructor: ";
+        var_dump(@unserialize('R:1;'));
+    }
+}
+
+class U {
+    public function __unserialize(array $data): void {}
+}
+
+var_dump(@unserialize('a:1:{i:0;O:1:"U":2:{i:0;O:1:"D":0:{}i:1;s:5:"bad";}}'));
+
+class P {}
+
+set_error_handler(function ($errno, $errstr) {
+    echo "$errstr: ";
+    var_dump(@unserialize('R:1;'));
+    return true;
+});
+var_dump(unserialize('O:1:"P":1:{s:3:"dyn";i:1;}') instanceof P);
+var_dump(unserialize('a:1:{i:0;O:8:"stdClass":0:{}}junk'));
+restore_error_handler();
+
+class N {
+    public function __toString(): string {
+        echo "__toString: ";
+        var_dump(unserialize('a:1:{i:0;s:6:"secret";}'));
+        return 'stdClass';
+    }
+}
+
+var_dump(@unserialize('R:2;', ['allowed_classes' => [new N]]));
+
+class S implements Serializable {
+    public $o;
+    public function serialize() { return ''; }
+    public function unserialize($data) { $this->o = unserialize($data); }
+    public function __serialize(): array { return []; }
+    public function __unserialize(array $data): void {}
+}
+
+$r = unserialize('a:2:{i:0;O:8:"stdClass":0:{}i:1;C:1:"S":4:{r:2;}}');
+echo "Serializable back-reference: ";
+var_dump($r[1]->o === $r[0]);
+?>
+--EXPECT--
+destructor: bool(false)
+bool(false)
+Creation of dynamic property P::$dyn is deprecated: bool(false)
+bool(true)
+unserialize(): Extra data starting at offset 29 of 33 bytes: bool(false)
+array(1) {
+  [0]=>
+  object(stdClass)#2 (0) {
+  }
+}
+__toString: array(1) {
+  [0]=>
+  string(6) "secret"
+}
+bool(false)
+Serializable back-reference: bool(true)

ext/standard/tests/serialize/gh24674_fiber.phpt

@@ -0,0 +1,42 @@
+--TEST--
+GH-24674 (Fiber switches are not allowed inside Serializable::unserialize())
+--FILE--
+<?php
+class S implements Serializable {
+    public function serialize() { return ''; }
+    public function unserialize($d) {
+        try { $f = new Fiber(function () { Fiber::suspend(); }); $f->start(); echo "start ok\n"; }
+        catch (FiberError $e) { echo "start: ", $e::class, ": ", $e->getMessage(), "\n"; }
+    }
+    public function __serialize(): array { return []; }
+    public function __unserialize(array $d): void {}
+}
+$g = new Fiber(function () { Fiber::suspend(); });
+$g->start();
+class R implements Serializable {
+    public function serialize() { return ''; }
+    public function unserialize($d) {
+        try { $GLOBALS['g']->resume(); echo "resume ok\n"; }
+        catch (FiberError $e) { echo "resume: ", $e::class, ": ", $e->getMessage(), "\n"; }
+    }
+    public function __serialize(): array { return []; }
+    public function __unserialize(array $d): void {}
+}
+unserialize('C:1:"S":0:{}');
+unserialize('C:1:"R":0:{}');
+$h = new Fiber(function () { unserialize('C:1:"T":0:{}'); });
+class T implements Serializable {
+    public function serialize() { return ''; }
+    public function unserialize($d) {
+        try { Fiber::suspend(); echo "suspend ok\n"; }
+        catch (FiberError $e) { echo "suspend: ", $e::class, ": ", $e->getMessage(), "\n"; }
+    }
+    public function __serialize(): array { return []; }
+    public function __unserialize(array $d): void {}
+}
+$h->start();
+?>
+--EXPECT--
+start: FiberError: Cannot switch fibers in current execution context
+resume: FiberError: Cannot switch fibers in current execution context
+suspend: FiberError: Cannot switch fibers in current execution context

ext/standard/tests/serialize/gh24674_gc.phpt

@@ -0,0 +1,39 @@
+--TEST--
+GH-24674 (GC destructor running during unserialize() gets its own context)
+--FILE--
+<?php
+class G {
+    public $self;
+    public static $hits = 0;
+    public function __destruct() {
+        if (self::$hits++ === 0) {
+            echo "destructor: ";
+            var_dump(@unserialize('R:3;'));
+        }
+    }
+}
+
+class U {
+    public function __unserialize(array $data): void {}
+}
+
+$n = 3000;
+$objs = '';
+$refs = '';
+for ($i = 0; $i < $n; $i++) {
+    $objs .= "i:$i;O:8:\"stdClass\":0:{}";
+    $refs .= "i:$i;r:" . (3 + $i) . ";";
+}
+$payload = 'a:2:{i:0;a:' . $n . ':{' . $objs . '}i:1;O:1:"U":' . ($n + 1) . ':{' . $refs . 'i:' . $n . ';X}}';
+
+for ($i = 0, $fill = 10000 - gc_status()['roots'] - 50; $i < $fill; $i++) {
+    $g = new G;
+    $g->self = $g;
+}
+unset($g);
+
+var_dump(@unserialize($payload));
+?>
+--EXPECT--
+destructor: bool(false)
+bool(false)

ext/standard/tests/serialize/gh24674_wakeup.phpt

@@ -0,0 +1,45 @@
+--TEST--
+GH-24674 (C: payload unserialized from __wakeup() or __unserialize())
+--FILE--
+<?php
+class A {}
+
+class S implements Serializable {
+    public $data;
+    public function serialize() { return serialize($this->data); }
+    public function unserialize($data) { $this->data = unserialize($data); }
+}
+
+function check($where) {
+    $p = 'x:i:0;a:1:{i:0;O:1:"A":0:{}};m:a:0:{}';
+    $r = unserialize('C:11:"ArrayObject":' . strlen($p) . ':{' . $p . '}', ['allowed_classes' => ['ArrayObject']]);
+    echo "$where allowed_classes: ", get_class($r[0]), "\n";
+
+    $o = new stdClass;
+    $s = new S;
+    $s->data = [$o, $o];
+    $r = unserialize(serialize($s));
+    echo "$where round trip: ";
+    var_dump($r->data[0] === $r->data[1]);
+}
+
+class W {
+    public function __wakeup() { check('__wakeup'); }
+}
+
+class U {
+    public function __unserialize(array $data): void { check('__unserialize'); }
+}
+
+unserialize('O:1:"W":0:{}');
+unserialize('O:1:"U":0:{}');
+check('top level');
+?>
+--EXPECTF--
+Deprecated: %s Serializable interface, which is deprecated. %s
+__wakeup allowed_classes: __PHP_Incomplete_Class
+__wakeup round trip: bool(true)
+__unserialize allowed_classes: __PHP_Incomplete_Class
+__unserialize round trip: bool(true)
+top level allowed_classes: __PHP_Incomplete_Class
+top level round trip: bool(true)

ext/standard/var.c

@@ -1473,6 +1473,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
 	zval *retval;
 	HashTable *class_hash = NULL, *prev_class_hash;
 	zend_long prev_max_depth, prev_cur_depth;
+	bool nested = BG(unserialize).data != NULL;
 
 	p = (const unsigned char*) buf;
 	PHP_VAR_UNSERIALIZE_INIT(var_hash);
@@ -1539,7 +1540,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
 		}
 	}
 
-	if (BG(unserialize).level > 1) {
+	if (nested) {
 		retval = var_tmp_var(&var_hash);
 	} else {
 		retval = return_value;
@@ -1549,7 +1550,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
 			php_error_docref(NULL, E_WARNING, "Error at offset " ZEND_LONG_FMT " of %zd bytes",
 				(zend_long)((char*)p - buf), buf_len);
 		}
-		if (BG(unserialize).level <= 1) {
+		if (!nested) {
 			zval_ptr_dtor(return_value);
 		}
 		RETVAL_FALSE;
@@ -1560,7 +1561,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
 					(zend_long)((char*)p - buf), buf_len);
 			}
 		}
-		if (BG(unserialize).level > 1) {
+		if (nested) {
 			ZVAL_COPY(return_value, retval);
 		} else if (Z_REFCOUNTED_P(return_value)) {
 			zend_refcounted *ref = Z_COUNTED_P(return_value);