From ba59dc324759c9564fe846177f9c6c3ca82cb7bf Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 20 Sep 2024 23:08:46 +0200 Subject: [PATCH 1/3] Soap: Split up an if condition into a nested if This is in preparation for adding functionality in later commits. --- ext/soap/soap.c | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 02ff10ae4d93a..f8bfb57ba6b02 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -1346,34 +1346,32 @@ PHP_METHOD(SoapServer, handle) zend_string *server = ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_SERVER); zend_is_auto_global(server); - if ((server_vars = zend_hash_find(&EG(symbol_table), server)) != NULL && - Z_TYPE_P(server_vars) == IS_ARRAY && - (encoding = zend_hash_str_find(Z_ARRVAL_P(server_vars), "HTTP_CONTENT_ENCODING", sizeof("HTTP_CONTENT_ENCODING")-1)) != NULL && - Z_TYPE_P(encoding) == IS_STRING) { - - if (zend_string_equals_literal(Z_STR_P(encoding), "gzip") - || zend_string_equals_literal(Z_STR_P(encoding), "x-gzip") - || zend_string_equals_literal(Z_STR_P(encoding), "deflate") - ) { - zval filter_params; - - array_init_size(&filter_params, 1); - add_assoc_long_ex(&filter_params, "window", sizeof("window")-1, 0x2f); /* ANY WBITS */ - - zf = php_stream_filter_create("zlib.inflate", &filter_params, 0); - zend_array_destroy(Z_ARR(filter_params)); - - if (zf) { - php_stream_filter_append(&SG(request_info).request_body->readfilters, zf); + if ((server_vars = zend_hash_find(&EG(symbol_table), server)) != NULL && Z_TYPE_P(server_vars) == IS_ARRAY) { + if ((encoding = zend_hash_str_find(Z_ARRVAL_P(server_vars), "HTTP_CONTENT_ENCODING", sizeof("HTTP_CONTENT_ENCODING")-1)) != NULL && Z_TYPE_P(encoding) == IS_STRING) { + if (zend_string_equals_literal(Z_STR_P(encoding), "gzip") + || zend_string_equals_literal(Z_STR_P(encoding), "x-gzip") + || zend_string_equals_literal(Z_STR_P(encoding), "deflate") + ) { + zval filter_params; + + array_init_size(&filter_params, 1); + add_assoc_long_ex(&filter_params, "window", sizeof("window")-1, 0x2f); /* ANY WBITS */ + + zf = php_stream_filter_create("zlib.inflate", &filter_params, 0); + zend_array_destroy(Z_ARR(filter_params)); + + if (zf) { + php_stream_filter_append(&SG(request_info).request_body->readfilters, zf); + } else { + php_error_docref(NULL, E_WARNING,"Can't uncompress compressed request"); + SOAP_SERVER_END_CODE(); + return; + } } else { - php_error_docref(NULL, E_WARNING,"Can't uncompress compressed request"); + php_error_docref(NULL, E_WARNING,"Request is compressed with unknown compression '%s'",Z_STRVAL_P(encoding)); SOAP_SERVER_END_CODE(); return; } - } else { - php_error_docref(NULL, E_WARNING,"Request is compressed with unknown compression '%s'",Z_STRVAL_P(encoding)); - SOAP_SERVER_END_CODE(); - return; } } From 882043542c2f98fb89220137f81c095ece409848 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 20 Sep 2024 23:36:41 +0200 Subject: [PATCH 2/3] Soap: Document how the current lookup functions work --- ext/soap/soap.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/soap/soap.c b/ext/soap/soap.c index f8bfb57ba6b02..2098c19a42ce9 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -3058,6 +3058,8 @@ static void deserialize_parameters(xmlNodePtr params, sdlFunctionPtr function, u } /* }}} */ +/* This function attempts to find the right function name based on the first node's name in the soap body. + * If that doesn't work it falls back to get_doc_function(). */ static sdlFunctionPtr find_function(sdlPtr sdl, xmlNodePtr func, zval* function_name) /* {{{ */ { sdlFunctionPtr function; @@ -4185,6 +4187,10 @@ static sdlFunctionPtr get_function(sdlPtr sdl, const char *function_name, size_t } /* }}} */ +/* This function tries to find the function that matches the given parameters. + * If params is NULL, it will return the first function that has no parameters. + * If params is not NULL, it will return the first function that has the same + * parameters as the given XML node. */ static sdlFunctionPtr get_doc_function(sdlPtr sdl, xmlNodePtr params) /* {{{ */ { if (sdl) { From a360f10bc37cf4b12d47cc2f4f69d024f7c1ec9b Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 21 Sep 2024 00:23:24 +0200 Subject: [PATCH 3/3] Fix #49169: SoapServer calls wrong function, although "SOAP action" header is correct Although the original reproducer no longer exists, I was able to cook up something similar. The problem is that there are two ways ext-soap currently looks up functions: 1) By matching the exact function name; but this doesn't work if the function name is not in the body. 2) By matching the parameter names. Neither of these work when we don't have the function name in the body, and when the parameter names are not unique. That's where we can use the "SOAPAction" header to distinguish between different actions. This header should be checked first and be matched against the "soapAction" attribute in the WSDL. We keep the existing fallbacks such that the chance of a BC break is minimized. Note that since #49169 a potential target namespace is ignored right now. --- ext/soap/soap.c | 65 +++++++++++++++++++++++++------ ext/soap/tests/bugs/bug49169.phpt | 44 +++++++++++++++++++++ ext/soap/tests/bugs/bug49169.wsdl | 49 +++++++++++++++++++++++ 3 files changed, 147 insertions(+), 11 deletions(-) create mode 100644 ext/soap/tests/bugs/bug49169.phpt create mode 100644 ext/soap/tests/bugs/bug49169.wsdl diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 2098c19a42ce9..710d72301b637 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -58,7 +58,7 @@ static sdlParamPtr get_param(sdlFunctionPtr function, const char *param_name, ze static sdlFunctionPtr get_function(sdlPtr sdl, const char *function_name, size_t function_name_length); static sdlFunctionPtr get_doc_function(sdlPtr sdl, xmlNodePtr params); -static sdlFunctionPtr deserialize_function_call(sdlPtr sdl, xmlDocPtr request, const char* actor, zval *function_name, uint32_t *num_params, zval **parameters, int *version, soapHeader **headers); +static sdlFunctionPtr deserialize_function_call(sdlPtr sdl, xmlDocPtr request, const char* actor, const char *soap_action, zval *function_name, uint32_t *num_params, zval **parameters, int *version, soapHeader **headers); static xmlDocPtr serialize_response_call(sdlFunctionPtr function, const char *function_name, const char *uri,zval *ret, soapHeader *headers, int version); static xmlDocPtr serialize_function_call(zval *this_ptr, sdlFunctionPtr function, const char *function_name, const char *uri, zval *arguments, uint32_t arg_count, int version, HashTable *soap_headers); static xmlNodePtr serialize_parameter(sdlParamPtr param,zval *param_val, uint32_t index,const char *name, int style, xmlNodePtr parent); @@ -1273,6 +1273,7 @@ PHP_METHOD(SoapServer, handle) HashTable *old_class_map, *old_typemap; int old_features; zval tmp_soap; + const char *soap_action = NULL; if (zend_parse_parameters(ZEND_NUM_ARGS(), "|s!", &arg, &arg_len) == FAILURE) { RETURN_THROWS(); @@ -1341,7 +1342,7 @@ PHP_METHOD(SoapServer, handle) if (!arg) { if (SG(request_info).request_body && 0 == php_stream_rewind(SG(request_info).request_body)) { - zval *server_vars, *encoding; + zval *server_vars, *encoding, *soap_action_z; php_stream_filter *zf = NULL; zend_string *server = ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_SERVER); @@ -1375,6 +1376,10 @@ PHP_METHOD(SoapServer, handle) } } + if ((soap_action_z = zend_hash_str_find(Z_ARRVAL_P(server_vars), ZEND_STRL("HTTP_SOAPACTION"))) != NULL && Z_TYPE_P(soap_action_z) == IS_STRING) { + soap_action = Z_STRVAL_P(soap_action_z); + } + doc_request = soap_xmlParseFile("php://input"); if (zf) { @@ -1418,7 +1423,7 @@ PHP_METHOD(SoapServer, handle) old_soap_version = SOAP_GLOBAL(soap_version); zend_try { - function = deserialize_function_call(service->sdl, doc_request, service->actor, &function_name, &num_params, ¶ms, &soap_version, &soap_headers); + function = deserialize_function_call(service->sdl, doc_request, service->actor, soap_action, &function_name, &num_params, ¶ms, &soap_version, &soap_headers); } zend_catch { /* Avoid leaking persistent memory */ xmlFreeDoc(doc_request); @@ -3090,6 +3095,37 @@ static sdlFunctionPtr find_function(sdlPtr sdl, xmlNodePtr func, zval* function_ } /* }}} */ +static sdlFunctionPtr find_function_using_soap_action(const sdl *sdl, const char *soap_action, zval* function_name) +{ + if (!sdl) { + return NULL; + } + + /* The soap action may be a http-quoted string, in which case we're removing the quotes here. */ + size_t soap_action_length = strlen(soap_action); + if (soap_action[0] == '"') { + if (soap_action_length < 2 || soap_action[soap_action_length - 1] != '"') { + return NULL; + } + soap_action++; + soap_action_length -= 2; + } + + /* TODO: This may depend on a particular target namespace, in which case this won't find a match when multiple different + * target namespaces are used until #45282 is resolved. */ + sdlFunctionPtr function; + ZEND_HASH_FOREACH_PTR(&sdl->functions, function) { + if (function->binding && function->binding->bindingType == BINDING_SOAP) { + sdlSoapBindingFunctionPtr fnb = function->bindingAttributes; + if (fnb && fnb->soapAction && strncmp(fnb->soapAction, soap_action, soap_action_length) == 0 && fnb->soapAction[soap_action_length] == '\0') { + ZVAL_STRING(function_name, function->functionName); + return function; + } + } + } ZEND_HASH_FOREACH_END(); + return NULL; +} + static xmlNodePtr get_envelope(xmlNodePtr trav, int *version, char **envelope_ns) { while (trav != NULL) { if (trav->type == XML_ELEMENT_NODE) { @@ -3115,12 +3151,12 @@ static xmlNodePtr get_envelope(xmlNodePtr trav, int *version, char **envelope_ns return NULL; } -static sdlFunctionPtr deserialize_function_call(sdlPtr sdl, xmlDocPtr request, const char* actor, zval *function_name, uint32_t *num_params, zval **parameters, int *version, soapHeader **headers) /* {{{ */ +static sdlFunctionPtr deserialize_function_call(sdlPtr sdl, xmlDocPtr request, const char* actor, const char *soap_action, zval *function_name, uint32_t *num_params, zval **parameters, int *version, soapHeader **headers) /* {{{ */ { char* envelope_ns = NULL; xmlNodePtr trav,env,head,body,func; xmlAttrPtr attr; - sdlFunctionPtr function; + sdlFunctionPtr function = NULL; encode_reset_ns(); @@ -3204,12 +3240,17 @@ static sdlFunctionPtr deserialize_function_call(sdlPtr sdl, xmlDocPtr request, c } trav = trav->next; } + if (soap_action) { + function = find_function_using_soap_action(sdl, soap_action, function_name); + } if (func == NULL) { - function = get_doc_function(sdl, NULL); - if (function != NULL) { - ZVAL_STRING(function_name, (char *)function->functionName); - } else { - soap_server_fault("Client", "looks like we got \"Body\" without function call", NULL, NULL, NULL); + if (!function) { + function = get_doc_function(sdl, NULL); + if (function) { + ZVAL_STRING(function_name, (char *)function->functionName); + } else { + soap_server_fault("Client", "looks like we got \"Body\" without function call", NULL, NULL, NULL); + } } } else { if (*version == SOAP_1_1) { @@ -3223,7 +3264,9 @@ static sdlFunctionPtr deserialize_function_call(sdlPtr sdl, xmlDocPtr request, c soap_server_fault("DataEncodingUnknown","Unknown Data Encoding Style", NULL, NULL, NULL); } } - function = find_function(sdl, func, function_name); + if (!function) { + function = find_function(sdl, func, function_name); + } if (sdl != NULL && function == NULL) { if (*version == SOAP_1_2) { soap_server_fault("rpc:ProcedureNotPresent","Procedure not present", NULL, NULL, NULL); diff --git a/ext/soap/tests/bugs/bug49169.phpt b/ext/soap/tests/bugs/bug49169.phpt new file mode 100644 index 0000000000000..5693eb2f751c2 --- /dev/null +++ b/ext/soap/tests/bugs/bug49169.phpt @@ -0,0 +1,44 @@ +--TEST-- +Bug #49169 (SoapServer calls wrong function, although "SOAP action" header is correct) +--EXTENSIONS-- +soap +--INI-- +soap.wsdl_cache_enabled=0 +--SKIPIF-- + +--POST-- + + + hello + + +--FILE-- +addfunction("test"); +$server->addfunction("test2"); +$_SERVER["HTTP_SOAPACTION"] = "#test"; +$server->handle(); +$_SERVER["HTTP_SOAPACTION"] = "#test2"; +$server->handle(); +?> +--EXPECT-- + +olleh + +5 diff --git a/ext/soap/tests/bugs/bug49169.wsdl b/ext/soap/tests/bugs/bug49169.wsdl new file mode 100644 index 0000000000000..27cd5858ad4f2 --- /dev/null +++ b/ext/soap/tests/bugs/bug49169.wsdl @@ -0,0 +1,49 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +