ext/soap: Fix SOAP classmap validation for integer keys

NEWS

@@ -51,6 +51,8 @@ PHP                                                                        NEWS
 - SOAP:
   . Fixed header injection through the Content-Type context option, the
     soapaction and the cookie names and values. (David Carlier)
+  . Fixed the SoapClient and SoapServer "classmap" option to reject arrays
+    containing integer keys. (Weilin Du, David Carlier)
 
 - MBString:
   . Fixed bug GH-22779 (mb_strrpos() returns the wrong position for a negative

UPGRADING

@@ -155,6 +155,9 @@ PHP 8.6 UPGRADE NOTES
     system.
 
 - SOAP:
+  . The "classmap" option of SoapClient and SoapServer now rejects arrays
+    containing integer keys. Previously, sparse integer-keyed and mixed-keyed
+    arrays could be accepted.
   . WSDL/XML Schema parsing now rejects out-of-range integer values for
     occurrence constraints and integer restriction facets. Negative minOccurs
     and maxOccurs values are rejected as well.

ext/soap/php_encoding.c

@@ -440,7 +440,7 @@ static xmlNodePtr master_to_xml_int(encodePtr encode, zval *data, int style, xml
 			zval *tmp;
 			zend_string *type_name;
 
-			ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(SOAP_GLOBAL(class_map), type_name, tmp) {
+			ZEND_HASH_FOREACH_STR_KEY_VAL(SOAP_GLOBAL(class_map), type_name, tmp) {
 				ZVAL_DEREF(tmp);
 				if (Z_TYPE_P(tmp) == IS_STRING &&
 				    ZSTR_LEN(ce->name) == Z_STRLEN_P(tmp) &&

ext/soap/soap.c

@@ -930,6 +930,18 @@ static HashTable* soap_create_typemap(sdlPtr sdl, HashTable *ht) /* {{{ */
 }
 /* }}} */
 
+static bool soap_class_map_has_only_string_keys(const HashTable *class_map)
+{
+	zend_string *key;
+	ZEND_HASH_FOREACH_STR_KEY(class_map, key) {
+		if (UNEXPECTED(key == NULL)) {
+			return false;
+		}
+	} ZEND_HASH_FOREACH_END();
+
+	return true;
+}
+
 /* {{{ SoapServer constructor */
 PHP_METHOD(SoapServer, __construct)
 {
@@ -1007,8 +1019,7 @@ PHP_METHOD(SoapServer, __construct)
 				zend_argument_type_error(2, "\"classmap\" option must be of type array, %s given", zend_zval_type_name(class_map_zv));
 				goto cleanup;
 			}
-			// TODO: this still accepts mixed keys arrays and not all numerically indexed arrays are packed
-			if (UNEXPECTED(HT_IS_PACKED(Z_ARRVAL_P(class_map_zv)))) {
+			if (UNEXPECTED(!soap_class_map_has_only_string_keys(Z_ARRVAL_P(class_map_zv)))) {
 				zend_argument_value_error(2, "\"classmap\" option must be an associative array");
 				goto cleanup;
 			}
@@ -2233,7 +2244,7 @@ PHP_METHOD(SoapClient, __construct)
 		}
 		if ((tmp = zend_hash_str_find(ht, "classmap", sizeof("classmap")-1)) != NULL &&
 			Z_TYPE_P(tmp) == IS_ARRAY) {
-			if (UNEXPECTED(HT_IS_PACKED(Z_ARRVAL_P(tmp)))) {
+			if (UNEXPECTED(!soap_class_map_has_only_string_keys(Z_ARRVAL_P(tmp)))) {
 				php_error_docref(NULL, E_ERROR, "'classmap' option must be an associative array");
 			}
 			ZVAL_COPY(Z_CLIENT_CLASSMAP_P(this_ptr), tmp);

ext/soap/tests/classmap_invalid_keys.phpt

@@ -0,0 +1,68 @@
+--TEST--
+SoapClient and SoapServer classmap options must only contain string keys
+--EXTENSIONS--
+soap
+--FILE--
+<?php
+
+$emptyHash = ['type' => 'stdClass'];
+unset($emptyHash['type']);
+
+$cases = [
+    'empty' => [],
+    'empty hash' => $emptyHash,
+    'packed' => ['stdClass'],
+    'sparse numeric' => [100 => 'stdClass'],
+    'numeric string' => ['1' => 'stdClass'],
+    'mixed' => ['type' => 'stdClass', 1 => 'stdClass'],
+    'associative' => ['type' => 'stdClass'],
+];
+
+foreach ($cases as $name => $classmap) {
+    echo "-- $name --\n";
+
+    try {
+        new SoapClient(null, [
+            'location' => 'http://example.com/',
+            'uri' => 'urn:test',
+            'classmap' => $classmap,
+        ]);
+        echo "SoapClient: OK\n";
+    } catch (Throwable $e) {
+        echo $e->getMessage(), "\n";
+    }
+
+    try {
+        new SoapServer(null, [
+            'uri' => 'urn:test',
+            'classmap' => $classmap,
+        ]);
+        echo "SoapServer: OK\n";
+    } catch (Throwable $e) {
+        echo $e->getMessage(), "\n";
+    }
+}
+
+?>
+--EXPECT--
+-- empty --
+SoapClient: OK
+SoapServer: OK
+-- empty hash --
+SoapClient: OK
+SoapServer: OK
+-- packed --
+SoapClient::__construct(): 'classmap' option must be an associative array
+SoapServer::__construct(): Argument #2 ($options) "classmap" option must be an associative array
+-- sparse numeric --
+SoapClient::__construct(): 'classmap' option must be an associative array
+SoapServer::__construct(): Argument #2 ($options) "classmap" option must be an associative array
+-- numeric string --
+SoapClient::__construct(): 'classmap' option must be an associative array
+SoapServer::__construct(): Argument #2 ($options) "classmap" option must be an associative array
+-- mixed --
+SoapClient::__construct(): 'classmap' option must be an associative array
+SoapServer::__construct(): Argument #2 ($options) "classmap" option must be an associative array
+-- associative --
+SoapClient: OK
+SoapServer: OK

ext/soap/tests/classmap_invalid_keys_error_types.phpt

@@ -0,0 +1,32 @@
+--TEST--
+SoapClient and SoapServer report an invalid classmap option differently
+--EXTENSIONS--
+soap
+--FILE--
+<?php
+
+$classmap = ['type' => 'stdClass', 1 => 'stdClass'];
+
+try {
+    new SoapClient(null, [
+        'location' => 'http://example.com/',
+        'uri' => 'urn:test',
+        'classmap' => $classmap,
+    ]);
+} catch (Throwable $e) {
+    echo 'SoapClient: ', $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+try {
+    new SoapServer(null, [
+        'uri' => 'urn:test',
+        'classmap' => $classmap,
+    ]);
+} catch (Throwable $e) {
+    echo 'SoapServer: ', $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+SoapClient: SoapFault: SoapClient::__construct(): 'classmap' option must be an associative array
+SoapServer: ValueError: SoapServer::__construct(): Argument #2 ($options) "classmap" option must be an associative array

ext/soap/tests/classmap_invalid_keys_exceptions_disabled.phpt

@@ -0,0 +1,18 @@
+--TEST--
+SoapClient reports an invalid classmap option as a fatal error when exceptions are disabled
+--EXTENSIONS--
+soap
+--FILE--
+<?php
+
+new SoapClient(null, [
+    'location' => 'http://example.com/',
+    'uri' => 'urn:test',
+    'exceptions' => false,
+    'classmap' => ['type' => 'stdClass', 1 => 'stdClass'],
+]);
+echo 'not reached', PHP_EOL;
+
+?>
+--EXPECTF--
+Fatal error: SoapClient::__construct(): 'classmap' option must be an associative array in %s on line %d

ext/soap/tests/classmap_private_property_packed_array.phpt

@@ -0,0 +1,45 @@
+--TEST--
+SoapClient must not read packed private classmap as string-keyed map
+--EXTENSIONS--
+soap
+--FILE--
+<?php
+
+class LocalSoapClient extends SoapClient {
+    public function __doRequest($request, $location, $action, $version, $one_way = false, ?string $uriParserClass = null): string {
+        echo "__doRequest called\n";
+
+        return <<<'XML'
+<?xml version="1.0" encoding="UTF-8"?>
+<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/">
+  <SOAP-ENV:Body>
+    <SOAP-ENV:Fault>
+      <faultcode>SOAP-ENV:Server</faultcode>
+      <faultstring>expected fault</faultstring>
+    </SOAP-ENV:Fault>
+  </SOAP-ENV:Body>
+</SOAP-ENV:Envelope>
+XML;
+    }
+}
+
+class Foo {}
+
+$client = new LocalSoapClient(null, [
+    'location' => 'http://example.org/',
+    'uri' => 'http://example.org/',
+]);
+
+$property = new ReflectionProperty(SoapClient::class, '_classmap');
+$property->setValue($client, ['Foo']);
+
+try {
+    $client->__soapCall('foo', [new Foo()]);
+} catch (SoapFault) {
+    echo "SOAP Fault thrown\n";
+}
+
+?>
+--EXPECT--
+__doRequest called
+SOAP Fault thrown