diff --git a/NEWS b/NEWS index a7f7caff7cee..4f8b896e70bf 100644 --- a/NEWS +++ b/NEWS @@ -96,6 +96,8 @@ PHP NEWS - SOAP: . Fixed the SOAP client accepting a response body shorter than its Content-Length and reusing the connection. (Ilia Alshanetsky) + . Fixed bug GH-24315 (Harden SoapClient cross-origin redirect handling). + (Ilia Alshanetsky) - SPL: . Fixed bug GH-23385 (SplDoublyLinkedList::serialize() use-after-free when diff --git a/UPGRADING b/UPGRADING index da184a86265d..85589f962823 100644 --- a/UPGRADING +++ b/UPGRADING @@ -27,6 +27,14 @@ PHP 8.7 UPGRADE NOTES not present in the result set. It previously reported the condition through PDO::ATTR_ERRMODE and returned false. +- SOAP: + . SoapClient no longer follows an HTTP redirect to a different origin + (scheme, host, or port) and throws a SoapFault instead. Previously the + request body and the configured credentials and headers were resent to + the new origin. A redirect from http to https on the same host is still + followed. Set the new follow_cross_origin_redirects option to true to + restore the previous behavior. + - Session: . Setting session.cookie_path to a value containing one of ";\r\n", or session.cookie_domain to a value containing one of ",; \t\r\n\013\014", diff --git a/ext/soap/php_http.c b/ext/soap/php_http.c index f20eccf3d04d..6856a1a069a1 100644 --- a/ext/soap/php_http.c +++ b/ext/soap/php_http.c @@ -347,6 +347,28 @@ static bool in_domain(const zend_string *host, const zend_string *domain) } } +static bool is_allowed_redirect(const php_uri *from, const php_uri *to) +{ + if (to->scheme == NULL || to->host == NULL) { + return true; + } + + if (!zend_string_equals_ci(from->host, to->host)) { + return false; + } + + zend_long to_port = to->port; + if (to_port == 0) { + to_port = zend_string_equals_literal(to->scheme, "https") ? 443 : 80; + } + + if (zend_string_equals(from->scheme, to->scheme)) { + return from->port == to_port; + } + + return zend_string_equals_literal(from->scheme, "http") && zend_string_equals_literal(to->scheme, "https") && to_port == 443; +} + bool make_http_soap_request( zval *this_ptr, zend_string *buf, zend_string *location, char *soapaction, int soap_version, zend_string *uri_parser_class, zval *return_value @@ -1191,6 +1213,15 @@ bool make_http_soap_request( } } } + + if (Z_TYPE_P(Z_CLIENT_FOLLOW_CROSS_ORIGIN_REDIRECTS_P(this_ptr)) != IS_TRUE && !is_allowed_redirect(uri, new_uri)) { + php_uri_struct_free(new_uri); + add_soap_fault(this_ptr, "HTTP", "Redirection to a different origin is not allowed", NULL, NULL, soap_lang_en); + smart_str_free(&soap_headers_z); + efree(http_msg); + return false; + } + uri = new_uri; if (--redirect_max < 1) { diff --git a/ext/soap/php_soap.h b/ext/soap/php_soap.h index d3c273615eb6..22fc913c07f4 100644 --- a/ext/soap/php_soap.h +++ b/ext/soap/php_soap.h @@ -255,6 +255,7 @@ static zend_always_inline zval *php_soap_deref(zval *zv) { #define Z_CLIENT_LAST_RESPONSE_P(zv) OBJ_PROP_NUM(Z_OBJ_P(zv), 32) #define Z_CLIENT_LAST_REQUEST_HEADERS_P(zv) OBJ_PROP_NUM(Z_OBJ_P(zv), 33) #define Z_CLIENT_LAST_RESPONSE_HEADERS_P(zv) OBJ_PROP_NUM(Z_OBJ_P(zv), 34) +#define Z_CLIENT_FOLLOW_CROSS_ORIGIN_REDIRECTS_P(zv) OBJ_PROP_NUM(Z_OBJ_P(zv), 35) typedef struct soap_url_object { php_uri *uri; diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 7266b06f8294..e573be247aa3 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -2325,6 +2325,12 @@ PHP_METHOD(SoapClient, __construct) ZVAL_FALSE(Z_CLIENT_KEEP_ALIVE_P(this_ptr)); } + if ((tmp = zend_hash_str_find(ht, "follow_cross_origin_redirects", sizeof("follow_cross_origin_redirects")-1)) != NULL && + (Z_TYPE_P(tmp) == IS_TRUE || + (Z_TYPE_P(tmp) == IS_LONG && Z_LVAL_P(tmp) == 1))) { + ZVAL_TRUE(Z_CLIENT_FOLLOW_CROSS_ORIGIN_REDIRECTS_P(this_ptr)); + } + if ((tmp = zend_hash_str_find(ht, "ssl_method", sizeof("ssl_method")-1)) != NULL && Z_TYPE_P(tmp) == IS_LONG) { ZVAL_LONG(Z_CLIENT_SSL_METHOD_P(this_ptr), Z_LVAL_P(tmp)); diff --git a/ext/soap/soap.stub.php b/ext/soap/soap.stub.php index fdd4a46e109f..bb324397845e 100644 --- a/ext/soap/soap.stub.php +++ b/ext/soap/soap.stub.php @@ -572,6 +572,7 @@ class SoapClient private ?string $__last_response = null; private ?string $__last_request_headers = null; private ?string $__last_response_headers = null; + private bool $_follow_cross_origin_redirects = false; public function __construct(?string $wsdl, array $options = []) {} diff --git a/ext/soap/soap_arginfo.h b/ext/soap/soap_arginfo.h index f6b146d779d6..3b2dce04a8cc 100644 --- a/ext/soap/soap_arginfo.h +++ b/ext/soap/soap_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit soap.stub.php instead. - * Stub hash: 14c74a5d6f547837f536920d5abb741e2b6e4373 */ + * Stub hash: dc9b0248aab9416e55d8b201fece34a9eb9b7f35 */ #include "zend_attributes.h" #include "zend_constants.h" @@ -748,5 +748,11 @@ static zend_class_entry *register_class_SoapClient(void) zend_declare_typed_property(class_entry, property___last_response_headers_name, &property___last_response_headers_default_value, ZEND_ACC_PRIVATE, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_STRING|MAY_BE_NULL)); zend_string_release_ex(property___last_response_headers_name, true); + zval property__follow_cross_origin_redirects_default_value; + ZVAL_FALSE(&property__follow_cross_origin_redirects_default_value); + zend_string *property__follow_cross_origin_redirects_name = zend_string_init("_follow_cross_origin_redirects", sizeof("_follow_cross_origin_redirects") - 1, true); + zend_declare_typed_property(class_entry, property__follow_cross_origin_redirects_name, &property__follow_cross_origin_redirects_default_value, ZEND_ACC_PRIVATE, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_BOOL)); + zend_string_release_ex(property__follow_cross_origin_redirects_name, true); + return class_entry; } diff --git a/ext/soap/tests/bugs/gh21421.phpt b/ext/soap/tests/bugs/gh21421.phpt index 1b8bb3be716f..d7292f3018dd 100644 --- a/ext/soap/tests/bugs/gh21421.phpt +++ b/ext/soap/tests/bugs/gh21421.phpt @@ -15,7 +15,7 @@ $client = new SoapClient(NULL, $options); var_dump($client); ?> --EXPECTF-- -object(SoapClient)#%d (35) { +object(SoapClient)#%d (36) { ["uri":"SoapClient":private]=> string(26) "http://schemas.nothing.com" ["style":"SoapClient":private]=> @@ -87,4 +87,6 @@ object(SoapClient)#%d (35) { NULL ["__last_response_headers":"SoapClient":private]=> NULL + ["_follow_cross_origin_redirects":"SoapClient":private]=> + bool(false) } diff --git a/ext/soap/tests/bugs/gh24315.phpt b/ext/soap/tests/bugs/gh24315.phpt new file mode 100644 index 000000000000..edb2720cb80f --- /dev/null +++ b/ext/soap/tests/bugs/gh24315.phpt @@ -0,0 +1,75 @@ +--TEST-- +GH-24315 (SoapClient follows redirects to a different origin) +--EXTENSIONS-- +soap +openssl +--SKIPIF-- + +--FILE-- +', + '', + 'ok', + ''; +} else { + http_response_code(302); + header("Location: " . base64_decode(substr($_SERVER["REQUEST_URI"], 1))); +} +PHP; + +php_cli_server_start($code, null, $args); + +$targets = [ + "different host" => "http://127.0.0.1:" . PHP_CLI_SERVER_PORT . "/ok", + "different port" => "http://" . PHP_CLI_SERVER_HOSTNAME . ":" . (PHP_CLI_SERVER_PORT + 1) . "/ok", + "protocol-relative different host" => "//127.0.0.1:" . PHP_CLI_SERVER_PORT . "/ok", + "https upgrade to a non-default port" => "https://" . PHP_CLI_SERVER_ADDRESS . "/ok", + "same origin" => "http://" . PHP_CLI_SERVER_ADDRESS . "/ok", + "same origin, host case differs" => "http://" . strtoupper(PHP_CLI_SERVER_HOSTNAME) . ":" . PHP_CLI_SERVER_PORT . "/ok", + "https upgrade on the same host" => "https://" . PHP_CLI_SERVER_HOSTNAME . "/ok", +]; + +$targets["different host, follow_cross_origin_redirects"] = $targets["different host"]; + +foreach ($targets as $label => $target) { + $client = new SoapClient(null, [ + 'location' => 'http://' . PHP_CLI_SERVER_ADDRESS . '/' . base64_encode($target), + 'uri' => 'test-uri', + 'login' => 'user', + 'password' => 'secret', + 'follow_cross_origin_redirects' => str_ends_with($label, 'follow_cross_origin_redirects'), + ]); + + $rejected = false; + try { + $client->__soapCall("foo", []); + } catch (SoapFault $e) { + $rejected = $e->getMessage() === "Redirection to a different origin is not allowed"; + } + echo $label, ": ", $rejected ? "rejected" : "not rejected", "\n"; +} +?> +--EXPECT-- +different host: rejected +different port: rejected +protocol-relative different host: rejected +https upgrade to a non-default port: rejected +same origin: not rejected +same origin, host case differs: not rejected +https upgrade on the same host: not rejected +different host, follow_cross_origin_redirects: not rejected