From 7e4a3236d92bc4bcad9ad6661d96f3da92d9c4d3 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 11 Oct 2023 00:11:16 +0200 Subject: [PATCH] Fix GH-12392: Segmentation fault on SoapClient::__getTypes There are two issues: - UAF because the hashmap resized while being iterated over, yet the local variables used internally in the macros are not updated. - The hashmap being iterated over is modified: entries are deleted after other entries have been added. This causes the deletion to fail sometimes because indices of buckets have shifted. Fix it by using a while loop iteration and HashPosition position tracker instead. Issue exists on PHP 8.1 too, but is much harder to trigger. The test file reproduces the issue reliably on PHP 8.2 and up. Closes GH-12409. --- NEWS | 4 +++ ext/soap/php_schema.c | 18 +++++++---- ext/soap/tests/gh12392.phpt | 28 +++++++++++++++++ ext/soap/tests/gh12392.wsdl | 61 +++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 ext/soap/tests/gh12392.phpt create mode 100644 ext/soap/tests/gh12392.wsdl diff --git a/NEWS b/NEWS index b5c21e37a67..0ba555c819b 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,10 @@ PHP NEWS . Fixed bug GH-8143 (Crashes in zend_accel_inheritance_cache_find since upgrading to 8.1.3 due to corrupt on-disk file cache). (turchanov) +- SOAP: + . Fixed bug GH-12392 (Segmentation fault on SoapClient::__getTypes). + (nielsdos) + 26 Oct 2023, PHP 8.1.25 - Core: diff --git a/ext/soap/php_schema.c b/ext/soap/php_schema.c index e93679a55ea..521c5d0f1d9 100644 --- a/ext/soap/php_schema.c +++ b/ext/soap/php_schema.c @@ -2261,17 +2261,23 @@ static void schema_type_fixup(sdlCtx *ctx, sdlTypePtr type) schema_content_model_fixup(ctx, type->model); } if (type->attributes) { - zend_string *str_key; - zend_ulong index; + HashPosition pos; + zend_hash_internal_pointer_reset_ex(type->attributes, &pos); - ZEND_HASH_FOREACH_KEY_PTR(type->attributes, index, str_key, attr) { - if (str_key) { + while ((attr = zend_hash_get_current_data_ptr_ex(type->attributes, &pos)) != NULL) { + zend_string *str_key; + zend_ulong index; + + if (zend_hash_get_current_key_ex(type->attributes, &str_key, &index, &pos) == HASH_KEY_IS_STRING) { schema_attribute_fixup(ctx, attr); + zend_result result = zend_hash_move_forward_ex(type->attributes, &pos); + ZEND_ASSERT(result == SUCCESS); } else { schema_attributegroup_fixup(ctx, attr, type->attributes); - zend_hash_index_del(type->attributes, index); + zend_result result = zend_hash_index_del(type->attributes, index); + ZEND_ASSERT(result == SUCCESS); } - } ZEND_HASH_FOREACH_END(); + } } } diff --git a/ext/soap/tests/gh12392.phpt b/ext/soap/tests/gh12392.phpt new file mode 100644 index 00000000000..8a234ba025b --- /dev/null +++ b/ext/soap/tests/gh12392.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-12392 (Segmentation fault on SoapClient::__getTypes) +--EXTENSIONS-- +soap +--FILE-- + WSDL_CACHE_NONE]); +echo 'Client created!' . "\n"; + +$types = $client->__getTypes(); +echo 'Got types!' . "\n"; + +var_dump($types); + +?> +--EXPECT-- +Client created! +Got types! +array(1) { + [0]=> + string(62) "struct dummy { + string foo; + string a; + string b; + string c; +}" +} diff --git a/ext/soap/tests/gh12392.wsdl b/ext/soap/tests/gh12392.wsdl new file mode 100644 index 00000000000..5e9a7ea094f --- /dev/null +++ b/ext/soap/tests/gh12392.wsdl @@ -0,0 +1,61 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +