Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null pointer dereference while serializing the response #9720

Closed
istoph opened this issue Oct 11, 2022 · 3 comments
Closed

Null pointer dereference while serializing the response #9720

istoph opened this issue Oct 11, 2022 · 3 comments

Comments

@istoph
Copy link

istoph commented Oct 11, 2022

Description

The following code:

<?php
class soapService
{
  function __construct() {
  }
  function openSession($user) {
      return ["OK", "200"]; //<-- The array leads into the area that leads to the error.
  }
}
$sc = new SoapServer(__dir__ .'/service.wsdl');
$sc->setClass("soapService");
$sc->handle();

with this service.wsdl file:

<?xml version="1.0" encoding="utf-8"?>
<definitions name="soapService" targetNamespace="urn:soapService" xmlns:typens="urn:soapService" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:soap="http://schemas.xmlsoap.org/wsdl/soap/" xmlns:soapenc="http://schemas.xmlsoap.org/soap/encoding/" xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/" xmlns="http://schemas.xmlsoap.org/wsdl/">
  <message name="openSession">
    <part name="user" type="xsd:string" />
  </message>
  <message name="openSessionResponse">
    <part name="status" type="xsd:string" />
    <part name="error_code" type="xsd:string" />
  </message>
  <portType name="soapServicePortType">
    <operation name="openSession">
      <documentation>Service Call: openSession</documentation>
      <input message="typens:openSession" />
      <output message="typens:openSessionResponse" />
    </operation>
  </portType>
  <binding name="soapServiceBinding" type="typens:soapServicePortType">
    <soap:binding style="rpc" transport="http://schemas.xmlsoap.org/soap/http" />
    <operation name="openSession">
      <soap:operation soapAction="urn:openSession" />
      <input>
        <soap:body namespace="urn:soapService" use="encoded" encodingStyle="http://schemas.xmlsoap.org/soap/encoding/" />
      </input>
      <output>
        <soap:body namespace="urn:soapService" use="encoded" encodingStyle="http://schemas.xmlsoap.org/soap/encoding/" />
      </output>
    </operation>
  </binding>
  <service name="soapServiceService">
    <port name="soapServicePort" binding="typens:soapServiceBinding">
      <soap:address location="###PHP_SELF###" />
    </port>
  </service>
</definitions>

Resulted in this a segfault with gdb output:

Program received signal SIGSEGV, Segmentation fault.
0x00007f60ce1f1c75 in __strlen_avx2 () from /lib64/libc.so.6
(gdb) bt
#0  0x00007f60ce1f1c75 in __strlen_avx2 () from /lib64/libc.so.6
#1  0x00007f60c1cdeb6d in get_param (function=function@entry=0x7f60cc2720c0, 
    param_name=param_name@entry=0x18 <error: Cannot access memory at address 0x18>, index=<optimized out>, response=response@entry=1)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/ext/soap/soap.c:3978
#2  0x00007f60c1cdfe54 in serialize_response_call2 (body=body@entry=0x557c1ae411d0, function=function@entry=0x7f60cc2720c0, 
    function_name=function_name@entry=0x7f60cc256078 "openSessionResponse", uri=uri@entry=0x7f60cc2660c0 "urn:soapService", 
    ret=ret@entry=0x7ffcc3881630, version=version@entry=1, main=1, node=0x0)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/ext/soap/soap.c:3292
#3  0x00007f60c1ce509b in serialize_response_call (function=0x7f60cc2720c0, function_name=0x7f60cc256078 "openSessionResponse", 
    uri=0x7f60cc2660c0 "urn:soapService", ret=0x7ffcc3881630, headers=0x0, version=1)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/ext/soap/soap.c:3660
#4  0x00007f60c1ced386 in zim_SoapServer_handle (execute_data=0x7f60cc213090, return_value=<optimized out>)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/ext/soap/soap.c:1484
#5  0x00007f60cbcbe4f5 in xdebug_execute_internal () from /opt/remi/php82/root/usr/lib64/php/modules/xdebug.so
#6  0x0000557c196a02c8 in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER ()
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/Zend/zend_vm_execute.h:1844
#7  execute_ex (ex=0x18) at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/Zend/zend_vm_execute.h:56047
#8  0x00007f60cbcbda4c in xdebug_execute_ex () from /opt/remi/php82/root/usr/lib64/php/modules/xdebug.so
#9  0x0000557c196a1932 in zend_execute (op_array=0x7f60cc280000, return_value=0x0)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/Zend/zend_vm_execute.h:60379
#10 0x0000557c1962ed15 in zend_execute_scripts (type=type@entry=8, retval=retval@entry=0x0, file_count=file_count@entry=3)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/Zend/zend.c:1780
#11 0x0000557c195c849a in php_execute_script (primary_file=<optimized out>) at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/main/main.c:2537
#12 0x0000557c1946e662 in main (argc=<optimized out>, argv=<optimized out>)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/sapi/fpm/fpm/fpm_main.c:1891
(gdb) frame 2
#2  0x00007f60c1cdfe54 in serialize_response_call2 (body=body@entry=0x557c1ae411d0, function=function@entry=0x7f60cc2720c0, 
    function_name=function_name@entry=0x7f60cc256078 "openSessionResponse", uri=uri@entry=0x7f60cc2660c0 "urn:soapService", 
    ret=ret@entry=0x7ffcc3881630, version=version@entry=1, main=1, node=0x0)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/ext/soap/soap.c:3292
3292				parameter = get_param(function, ZSTR_VAL(param_name), param_index, TRUE);
(gdb) info locals 
_z = 0x557c1b8bf740
__ht = 0x557c1b8bf700
__key = 0x0
_idx = 1
_count = 2
__h = <optimized out>
_size = <optimized out>
__z = 0x557c1b8bf750
data = 0x557c1b8bf740
i = 0
param_name = 0x0
param_index = <optimized out>
method = <optimized out>
param = <optimized out>
parameter = <optimized out>
param_count = <optimized out>
style = <optimized out>
use = <optimized out>
ns = <optimized out>
(gdb) frame 1
#1  0x00007f60c1cdeb6d in get_param (function=function@entry=0x7f60cc2720c0, 
    param_name=param_name@entry=0x18 <error: Cannot access memory at address 0x18>, index=<optimized out>, response=response@entry=1)
    at /usr/src/debug/php82-php-8.2.0~rc3-16.el8.remi.x86_64/ext/soap/soap.c:3978
3978			if ((tmp = zend_hash_str_find_ptr(ht, param_name, strlen(param_name))) != NULL) {
(gdb) info locals 
tmp = <optimized out>
ht = 0x7f60cc260428

In frame 2 the var param_name is from type zend_string but in frame 1 a char* is expected. The type conflict leads to 0x18 not being recognized as null in the if statement before.

But I expected this output instead:

<?xml version="1.0" encoding="UTF-8"?>
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:ns1="urn:soapService" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/" SOAP-ENV:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"><SOAP-ENV:Body><ns1:openSessionResponse><status xsi:type="xsd:string">OK</status><error_code xsi:type="xsd:string">200</error_code></ns1:openSessionResponse></SOAP-ENV:Body></SOAP-ENV:Envelope>

After we call it with:

curl -L --request POST --data '<?xml version="1.0" encoding="UTF-8"?>
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:ns1="urn:soapService" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/s
<SOAP-ENV:Body><ns1:openSession><user xsi:type="xsd:string">istoph</user></ns1:openSession></SOAP-ENV:Body></SOAP-ENV:Envelope>' http://localhost:8080/soap/ -H "Content-Type: text/xml; charset=UTF-8"

In version PHP 5.6.32 this example still works. Between PHP 7.0 and 8.2 we could see the failure.

PHP Version

PHP 8.2.0

Operating System

Rhel 8

@istoph
Copy link
Author

istoph commented Oct 11, 2022

We have been able to fix this on our site with the following patch. We do not know if this patch is also suitable for upstream.

--- php-src/ext/soap/soap.c	2022-10-10 11:35:34.000000000 +0200
+++ soap.c	2022-10-10 18:22:04.000000000 +0200
@@ -3791,15 +3791,15 @@
 	} else if (param_count > 1 && Z_TYPE_P(ret) == IS_ARRAY) {
 		zval *data;
 		int i = 0;
-		zend_string *param_name;
+		char *param_name;
 		zend_ulong param_index = i;
 
 		ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(ret), param_index, param_name, data) {
-			parameter = get_param(function, ZSTR_VAL(param_name), param_index, TRUE);
+			parameter = get_param(function, param_name, param_index, TRUE);
 			if (style == SOAP_RPC) {
-				param = serialize_parameter(parameter, data, i, ZSTR_VAL(param_name), use, method);
+				param = serialize_parameter(parameter, data, i, param_name, use, method);
 			} else {
-				param = serialize_parameter(parameter, data, i, ZSTR_VAL(param_name), use, body);
+				param = serialize_parameter(parameter, data, i, param_name, use, body);
 				if (function && function->binding->bindingType == BINDING_SOAP) {
 					if (parameter && parameter->element) {
 						ns = encode_add_ns(param, parameter->element->namens);

@cmb69
Copy link
Member

cmb69 commented Oct 12, 2022

Thanks for reporting! I can confirm the issue, but your patch wouldn't even compile for me, since ZEND_HASH_FOREACH_KEY_VAL's param_name is supposed to be a zend_string* (not a char*). Did you perhaps also replace ZEND_HASH_FOREACH_KEY_VAL with some other macro which yields a char*? I'm not sure if there is even something like this, so I worked around:

 ext/soap/soap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ext/soap/soap.c b/ext/soap/soap.c
index d5731a5c9b..fbf6546beb 100644
--- a/ext/soap/soap.c
+++ b/ext/soap/soap.c
@@ -3360,11 +3360,11 @@ static int serialize_response_call2(xmlNodePtr body, sdlFunctionPtr function, ch
 		zend_ulong param_index = i;
 
 		ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(ret), param_index, param_name, data) {
-			parameter = get_param(function, ZSTR_VAL(param_name), param_index, TRUE);
+			parameter = get_param(function, param_name ? ZSTR_VAL(param_name) : NULL, param_index, TRUE);
 			if (style == SOAP_RPC) {
-				param = serialize_parameter(parameter, data, i, ZSTR_VAL(param_name), use, method);
+				param = serialize_parameter(parameter, data, i, param_name ? ZSTR_VAL(param_name) : NULL, use, method);
 			} else {
-				param = serialize_parameter(parameter, data, i, ZSTR_VAL(param_name), use, body);
+				param = serialize_parameter(parameter, data, i, param_name ? ZSTR_VAL(param_name) : NULL, use, body);
 				if (function && function->binding->bindingType == BINDING_SOAP) {
 					if (parameter && parameter->element) {
 						ns = encode_add_ns(param, parameter->element->namens);

@istoph
Copy link
Author

istoph commented Oct 13, 2022

@cmb69 Thank you for looking at the bug! I have applied the change you customized to our test environment and all unit and integration tests are running. So this patch would be a great help for us.

cmb69 added a commit to cmb69/php-src that referenced this issue Oct 13, 2022
When traversing the result array, we need to cater to `param_name`
possibly being `NULL`.  Prior to PHP 7.0.0, this was implicitly done
because `param_name` was of type `char*`.
@cmb69 cmb69 changed the title segfault in php-soap, type conflict with zend_string and char Null pointer dereference while serializing the response Oct 13, 2022
@cmb69 cmb69 closed this as completed in e440e37 Oct 13, 2022
cmb69 added a commit that referenced this issue Oct 13, 2022
* PHP-8.0:
  Fix GH-9720: Null pointer dereference while serializing the response
cmb69 added a commit that referenced this issue Oct 13, 2022
* PHP-8.1:
  Fix GH-9720: Null pointer dereference while serializing the response
cmb69 added a commit that referenced this issue Oct 13, 2022
* PHP-8.2:
  Fix GH-9720: Null pointer dereference while serializing the response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@cmb69 @istoph and others