ext/standard: Improve checking of allowed_classes option

ext/standard/tests/serialize/unserialize_allowed_classes_option_invalid_array.phpt

@@ -0,0 +1,61 @@
+--TEST--
+Test unserialize() with array allowed_classes and nonsensical values
+--FILE--
+<?php
+class foo {
+        public $x = "bar";
+}
+$z = array(new foo(), 2, "3");
+$s = serialize($z);
+
+try {
+    unserialize($s, ["allowed_classes" => [null]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => [false]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => [true]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => [42]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => [15.2]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => [[]]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => [STDERR]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => [new stdClass]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+TypeError: unserialize(): Option "allowed_classes" must be an array of class names
+TypeError: unserialize(): Option "allowed_classes" must be an array of class names
+TypeError: unserialize(): Option "allowed_classes" must be an array of class names
+TypeError: unserialize(): Option "allowed_classes" must be an array of class names
+TypeError: unserialize(): Option "allowed_classes" must be an array of class names
+TypeError: unserialize(): Option "allowed_classes" must be an array of class names
+TypeError: unserialize(): Option "allowed_classes" must be an array of class names
+Error: Object of class stdClass could not be converted to string

ext/standard/tests/serialize/unserialize_allowed_classes_option_invalid_class_names.phpt

@@ -0,0 +1,48 @@
+--TEST--
+Test unserialize() with array allowed_classes and nonsensical class names
+--FILE--
+<?php
+class foo {
+        public $x = "bar";
+}
+$z = array(new foo(), 2, "3");
+$s = serialize($z);
+
+try {
+    unserialize($s, ["allowed_classes" => [""]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => ["245blerg"]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => ["  whitespace  "]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => ["name\nwith whitespace"]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => ['$dollars']]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+try {
+    unserialize($s, ["allowed_classes" => ["have\0nul_byte"]]);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+ValueError: unserialize(): Option "allowed_classes" must be an array of class names, "  whitespace  " given
+ValueError: unserialize(): Option "allowed_classes" must be an array of class names, "name
+with whitespace" given
+ValueError: unserialize(): Option "allowed_classes" must be an array of class names, "$dollars" given
+ValueError: unserialize(): Option "allowed_classes" must be an array of class names, "have" given

ext/standard/var.c

@@ -1370,25 +1370,33 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co
 			goto cleanup;
 		}
 
-		if(classes && (Z_TYPE_P(classes) == IS_ARRAY || !zend_is_true(classes))) {
+		if (classes && (Z_TYPE_P(classes) == IS_ARRAY || !zend_is_true(classes))) {
 			ALLOC_HASHTABLE(class_hash);
 			zend_hash_init(class_hash, (Z_TYPE_P(classes) == IS_ARRAY)?zend_hash_num_elements(Z_ARRVAL_P(classes)):0, NULL, NULL, 0);
 		}
-		if(class_hash && Z_TYPE_P(classes) == IS_ARRAY) {
+		if (class_hash && Z_TYPE_P(classes) == IS_ARRAY) {
 			zval *entry;
-			zend_string *lcname;
 
 			ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(classes), entry) {
-				convert_to_string(entry);
-				lcname = zend_string_tolower(Z_STR_P(entry));
+				ZVAL_DEREF(entry);
+				if (UNEXPECTED(Z_TYPE_P(entry) != IS_STRING && Z_TYPE_P(entry) != IS_OBJECT)) {
+					zend_type_error("%s(): Option \"allowed_classes\" must be an array of class names", function_name);
+					goto cleanup;
+				}
+				zend_string *name = zval_try_get_string(entry);
+				if (UNEXPECTED(name == NULL)) {
+					goto cleanup;
+				}
+				if (UNEXPECTED(!zend_is_valid_class_name(name))) {
+					zend_value_error("%s(): Option \"allowed_classes\" must be an array of class names, \"%s\" given", function_name, ZSTR_VAL(name));
+					zend_string_release_ex(name, false);
+					goto cleanup;
+				}
+				zend_string *lcname = zend_string_tolower(name);
 				zend_hash_add_empty_element(class_hash, lcname);
-		        zend_string_release_ex(lcname, 0);
+		        zend_string_release_ex(name, false);
+		        zend_string_release_ex(lcname, false);
 			} ZEND_HASH_FOREACH_END();
-
-			/* Exception during string conversion. */
-			if (EG(exception)) {
-				goto cleanup;
-			}
 		}
 		php_var_unserialize_set_allowed_classes(var_hash, class_hash);