diff --git a/NEWS b/NEWS index 484e462698d9..9ee584efb728 100644 --- a/NEWS +++ b/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 diff --git a/UPGRADING b/UPGRADING index 93245f87f42a..494bc2208e6c 100644 --- a/UPGRADING +++ b/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. diff --git a/ext/soap/php_encoding.c b/ext/soap/php_encoding.c index d08c5a683189..72de145e7b04 100644 --- a/ext/soap/php_encoding.c +++ b/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) && diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 171b529b3355..0edb5fcfae13 100644 --- a/ext/soap/soap.c +++ b/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); diff --git a/ext/soap/tests/classmap_invalid_keys.phpt b/ext/soap/tests/classmap_invalid_keys.phpt new file mode 100644 index 000000000000..681a59a5fc51 --- /dev/null +++ b/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-- + '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 diff --git a/ext/soap/tests/classmap_invalid_keys_error_types.phpt b/ext/soap/tests/classmap_invalid_keys_error_types.phpt new file mode 100644 index 000000000000..1eb190bdbfff --- /dev/null +++ b/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-- + '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 diff --git a/ext/soap/tests/classmap_invalid_keys_exceptions_disabled.phpt b/ext/soap/tests/classmap_invalid_keys_exceptions_disabled.phpt new file mode 100644 index 000000000000..0d80b132e3c5 --- /dev/null +++ b/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-- + '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 diff --git a/ext/soap/tests/classmap_private_property_packed_array.phpt b/ext/soap/tests/classmap_private_property_packed_array.phpt new file mode 100644 index 000000000000..058483d2dc27 --- /dev/null +++ b/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-- + + + + + SOAP-ENV:Server + expected fault + + + +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