Fix extra data detection for nested unserialize in Serializable interface

Author: TimWolla

ext/standard/tests/serialize/unserialize_extra_data.phpt renamed to ext/standard/tests/serialize/unserialize_extra_data_001.phpt

@@ -1,5 +1,5 @@
 --TEST--
-Extra data at the end of a valid value
+Test unserialize() with extra data at the end of a valid value
 --FILE--
 <?php
 

ext/standard/tests/serialize/unserialize_extra_data_002.phpt

@@ -0,0 +1,42 @@
+--TEST--
+Test unserialize() with extra data at the end of a valid value with nested unserialize
+--FILE--
+<?php
+
+final class Foo {
+    public $foo;
+
+    public function __unserialize(array $foo)
+    {
+        $this->foo = unserialize($foo['bar']);
+    }
+
+    public function __serialize(): array
+    {
+        return [
+            'bar' => serialize($this->foo) . 'garbage',
+        ];
+    }
+}
+
+$f = new Foo;
+$f->foo = ['a', 'b', 'c'];
+
+var_dump(unserialize(serialize($f) . 'garbage'));
+
+?>
+--EXPECTF--
+Warning: unserialize(): Extra data starting at offset 81 of 88 bytes in %s on line %d
+
+Warning: unserialize(): Extra data starting at offset 42 of 49 bytes in %s on line %d
+object(Foo)#2 (1) {
+  ["foo"]=>
+  array(3) {
+    [0]=>
+    string(1) "a"
+    [1]=>
+    string(1) "b"
+    [2]=>
+    string(1) "c"
+  }
+}

ext/standard/tests/serialize/unserialize_extra_data_003.phpt

@@ -0,0 +1,42 @@
+--TEST--
+Test unserialize() with extra data at the end of a valid value with Serializable
+--FILE--
+<?php
+
+final class Foo implements Serializable {
+    public $foo;
+
+    public function unserialize(string $foo)
+    {
+        $this->foo = unserialize($foo);
+    }
+
+    public function serialize(): string
+    {
+        return serialize($this->foo) . 'garbage';
+    }
+}
+
+$f = new Foo;
+$f->foo = ['a', 'b', 'c'];
+
+var_dump(unserialize(serialize($f) . 'garbage'));
+
+?>
+--EXPECTF--
+Deprecated: Foo implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in %s on line %d
+
+Warning: unserialize(): Extra data starting at offset 42 of 49 bytes in %s on line %d
+
+Warning: unserialize(): Extra data starting at offset 64 of 71 bytes in %s on line %d
+object(Foo)#2 (1) {
+  ["foo"]=>
+  array(3) {
+    [0]=>
+    string(1) "a"
+    [1]=>
+    string(1) "b"
+    [2]=>
+    string(1) "c"
+  }
+}

ext/standard/var.c

@@ -1407,16 +1407,19 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
 			zval_ptr_dtor(return_value);
 		}
 		RETVAL_FALSE;
-	} else if ((char*)p < buf + buf_len) {
-		if (!EG(exception)) {
-			php_error_docref(NULL, E_WARNING, "Extra data starting at offset " ZEND_LONG_FMT " of %zd bytes",
-				(zend_long)((char*)p - buf), buf_len);
+	} else {
+		if ((char*)p < buf + buf_len) {
+			if (!EG(exception)) {
+				php_error_docref(NULL, E_WARNING, "Extra data starting at offset " ZEND_LONG_FMT " of %zd bytes",
+					(zend_long)((char*)p - buf), buf_len);
+			}
+		}
+		if (BG(unserialize).level > 1) {
+			ZVAL_COPY(return_value, retval);
+		} else if (Z_REFCOUNTED_P(return_value)) {
+			zend_refcounted *ref = Z_COUNTED_P(return_value);
+			gc_check_possible_root(ref);
 		}
-	} else if (BG(unserialize).level > 1) {
-		ZVAL_COPY(return_value, retval);
-	} else if (Z_REFCOUNTED_P(return_value)) {
-		zend_refcounted *ref = Z_COUNTED_P(return_value);
-		gc_check_possible_root(ref);
 	}
 
 cleanup: