Segmentation fault on SoapClient::__getTypes

[gdahlke-adv]

Description

We run into a segmentation fault when calling SoapClient::__getTypes() using the WSDL listed below on PHP versions 8.2 and 8.3 (versions 7.4, 8.0 and 8.1 behave as expected, see below).

Similar to the following bug (although different PHP versions are affected): https://bugs.php.net/bug.php?id=81154

The following code:

<?php
echo 'Loading this WSDL will take some time...' . "\n";

$client = new SoapClient('https://xzufi-v2-2-0-ni.zfinder.de/?wsdl');
echo 'Client created!' . "\n";

$types = $client->__getTypes();
echo 'Got types!' . "\n";

Resulted in this output:

Loading this WSDL will take some time...
Client created!
Segmentation fault

But we expected this output instead:

Loading this WSDL will take some time...
Client created!
Got types!

Tested with different PHP versions via docker:

# Install Soap extension: apk add libxml2-dev && docker-php-ext-install soap

php:7.4-alpine - OK
php:8.0-alpine - OK
php:8.1-alpine - OK
php:8.2-alpine - Segmentation fault
php:8.3.0RC3-alpine - Segmentation fault

GDB Backtrace on Debian 12 (bookworm) with PHP 8.2.10 (./configure --enable-debug --enable-soap):

(gdb) run test.php
Starting program: /php-8.2.10/sapi/cli/php test.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
__strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
76      ../sysdeps/x86_64/multiarch/strlen-avx2.S: No such file or directory.
(gdb) bt
#0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
#1  0x0000555555849987 in smart_str_appends (dest=0x7fffffffc0d0, src=0x0) at /php-8.2.10/Zend/zend_smart_str.h:176
#2  0x000055555585e151 in type_to_string (type=0x7ffff5585f00, buf=0x7fffffffc0d0, level=0) at /php-8.2.10/ext/soap/soap.c:4321
#3  0x0000555555856b17 in zim_SoapClient___getTypes (execute_data=0x7ffff5415110, return_value=0x7ffff5415090) at /php-8.2.10/ext/soap/soap.c:2528
#4  0x0000555555abe3c7 in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER () at /php-8.2.10/Zend/zend_vm_execute.h:1951
#5  0x0000555555b3181f in execute_ex (ex=0x7ffff5415020) at /php-8.2.10/Zend/zend_vm_execute.h:56080
#6  0x0000555555b35f7c in zend_execute (op_array=0x7ffff5483000, return_value=0x0) at /php-8.2.10/Zend/zend_vm_execute.h:60408
#7  0x0000555555a7d956 in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /php-8.2.10/Zend/zend.c:1833
#8  0x00005555559e0c92 in php_execute_script (primary_file=0x7fffffffe970) at /php-8.2.10/main/main.c:2542
#9  0x0000555555bedac1 in do_cli (argc=2, argv=0x5555568380e0) at /php-8.2.10/sapi/cli/php_cli.c:964
#10 0x0000555555bee65c in main (argc=2, argv=0x5555568380e0) at /php-8.2.10/sapi/cli/php_cli.c:1333

Any help will be greatly appreciated!

PHP Version

PHP 8.2.11

Operating System

No response

[devnexen]

reproduced even with the master branch.

[Girgias]

attr->name is NULL for some reason.

[nielsdos]

I can reproduce this as well, weird that the attribute name would be NULL, that's not supposed to happen.
I'll run a bisect.
EDIT: I'm pretty sure this broke when PHP 8.2 started using the new HashTable array structures. The bug here is actually a UAF because the array over which is iterated is also modified, so a resize may happen in schema_attributegroup_fixup.
EDIT2: and after fixing that I do get the NULL pointer deref.

[nielsdos]

There are two issues at play here:

• 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. This is why y'all got NULL derefs: something should've been deleted but wasn't. Extra fun is the fact that there is recursion at play, so it's hard to predict how many elements are gonna be added.
• I got a UAF because the hashmap resized while being iterated over, yet the local variables used internally in the iteration variables where not updated.

Here's what I came up with: use two arrays, the original one and the resulting one after transformation. I'd have to benchmark the performance difference though...
I also think that 8.1 is technically also affected, but the way hashtables work is different between 8.1 and 8.2, so the issue might not be easy to expose there. EDIT: confirmed 8.1 is affected too.

diff --git a/ext/soap/php_schema.c b/ext/soap/php_schema.c
index 68d78326e3..edcc6c4743 100644
--- a/ext/soap/php_schema.c
+++ b/ext/soap/php_schema.c
@@ -2261,17 +2261,21 @@ 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_ASSERT(zend_hash_move_forward_ex(type->attributes, &pos) == SUCCESS);
 			} else {
 				schema_attributegroup_fixup(ctx, attr, type->attributes);
-				zend_hash_index_del(type->attributes, index);
+				ZEND_ASSERT(zend_hash_index_del(type->attributes, index) == SUCCESS);
 			}
-		} ZEND_HASH_FOREACH_END();
+		}
 	}
 }
 

[nielsdos]

I updated the patch to what should be final, and confirmed that the __getTypes() output is as expected by diffing it.
I'm not sure if I'm going to be able to reduce the wsdl file to a minimum reproducer though, but I'll give it a shot.
Hopefully PR today or tomorrow.

[gdahlke-adv]

Thank you very much @devnexen @Girgias @nielsdos for your very fast response and development of the fix and PR!