Skip to content

Segmentation fault on SoapClient::__getTypes #12392

Closed

Description

@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

Activity

  1. devnexen commented on Oct 9, 2023

    @devnexen
    Member

    reproduced even with the master branch.

  2. Girgias commented on Oct 9, 2023

    @Girgias
    Member

    attr->name is NULL for some reason.

  3. ndossche commented on Oct 9, 2023

    @ndossche
    Member

    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.

  4. ndossche commented on Oct 9, 2023

    @ndossche
    Member

    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();
    +		}
     	}
     }
     
  5. ndossche commented on Oct 10, 2023

    @ndossche
    Member

    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.

  6. added a commit that references this issue on Oct 10, 2023
    cca9385
  7. gdahlke-adv commented on Oct 11, 2023

    @gdahlke-adv
    Author

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

  8. added 3 commits that reference this issue on Oct 11, 2023
    7e4a323
    d8cd0f4
    124c812
  9. added a commit that references this issue on Oct 11, 2023
    0ba24a5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions