From 43e63168e920af5ba504c6b8e98678b9dc6a991e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 18 Oct 2023 22:20:50 +0200 Subject: [PATCH 1/3] Fix bug #66150: SOAP WSDL cache race condition causes Segmentation Fault When we have two processes both trying to cache a WSDL, they might start writing the data to the same temporary file, causing file corruption due to the race condition. Fix this by creating a temporary file first, and then moving it to the final location. If moving fails then we know another process finished caching first. This also fixes #67617 as a consequence of its implementation. Closes GH-12469. --- NEWS | 3 +++ ext/soap/php_sdl.c | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 521972251c2..90570d34231 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,9 @@ PHP NEWS - SOAP: . Fixed bug GH-12392 (Segmentation fault on SoapClient::__getTypes). (nielsdos) + . Fixed bug #66150 (SOAP WSDL cache race condition causes Segmentation + Fault). (nielsdos) + . Fixed bug #67617 (SOAP leaves incomplete cache file on ENOSPC). (nielsdos) - XSL: . Add missing module dependency. (nielsdos) diff --git a/ext/soap/php_sdl.c b/ext/soap/php_sdl.c index 3dd8e6c5d76..1848d5ecda3 100644 --- a/ext/soap/php_sdl.c +++ b/ext/soap/php_sdl.c @@ -22,6 +22,7 @@ #include "ext/standard/md5.h" #include "zend_virtual_cwd.h" +#include "main/php_open_temporary_file.h" #include #include @@ -2119,7 +2120,10 @@ static void add_sdl_to_cache(const char *fn, const char *uri, time_t t, sdlPtr s HashTable tmp_bindings; HashTable tmp_functions; - f = open(fn,O_CREAT|O_WRONLY|O_EXCL|O_BINARY,S_IREAD|S_IWRITE); + /* To avoid race conditions, we first create a temporary file and then rename it atomically + * at the end of the function. (see bug #66150) */ + zend_string *temp_file_path; + f = php_open_temporary_fd_ex(SOAP_GLOBAL(cache_dir), "tmp.wsdl.", &temp_file_path, PHP_TMP_FILE_SILENT); if (f < 0) {return;} @@ -2371,13 +2375,21 @@ static void add_sdl_to_cache(const char *fn, const char *uri, time_t t, sdlPtr s } ZEND_HASH_FOREACH_END(); } - php_ignore_value(write(f, ZSTR_VAL(buf.s), ZSTR_LEN(buf.s))); + bool valid_file = write(f, ZSTR_VAL(buf.s), ZSTR_LEN(buf.s)) == ZSTR_LEN(buf.s); close(f); + + /* Make sure that incomplete files (e.g. due to disk space issues, see bug #66150) are not utilised. */ + if (valid_file) { + /* This is allowed to fail, this means that another process was raced to create the file. */ + (void) VCWD_RENAME(ZSTR_VAL(temp_file_path), fn); + } + smart_str_free(&buf); zend_hash_destroy(&tmp_functions); zend_hash_destroy(&tmp_bindings); zend_hash_destroy(&tmp_encoders); zend_hash_destroy(&tmp_types); + zend_string_release_ex(temp_file_path, false); } From abf562c4172ec3c52b07006474e1cd536e6b6e50 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 19 Oct 2023 16:41:29 +0200 Subject: [PATCH 2/3] Fix incorrect uri check in SOAP caching If i == 0 then the check will compare 0 bytes. We are supposed to check if the uri is identical. Closes GH-12479. --- NEWS | 1 + ext/soap/php_sdl.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 90570d34231..defce40d786 100644 --- a/NEWS +++ b/NEWS @@ -27,6 +27,7 @@ PHP NEWS . Fixed bug #66150 (SOAP WSDL cache race condition causes Segmentation Fault). (nielsdos) . Fixed bug #67617 (SOAP leaves incomplete cache file on ENOSPC). (nielsdos) + . Fix incorrect uri check in SOAP caching. (nielsdos) - XSL: . Add missing module dependency. (nielsdos) diff --git a/ext/soap/php_sdl.c b/ext/soap/php_sdl.c index 1848d5ecda3..e7ea1123178 100644 --- a/ext/soap/php_sdl.c +++ b/ext/soap/php_sdl.c @@ -1537,7 +1537,7 @@ static HashTable* sdl_deserialize_parameters(encodePtr *encoders, sdlTypePtr *ty return ht; } -static sdlPtr get_sdl_from_cache(const char *fn, const char *uri, time_t t, time_t *cached) +static sdlPtr get_sdl_from_cache(const char *fn, const char *uri, size_t uri_len, time_t t, time_t *cached) { sdlPtr sdl; time_t old_t; @@ -1584,7 +1584,7 @@ static sdlPtr get_sdl_from_cache(const char *fn, const char *uri, time_t t, time *cached = old_t; WSDL_CACHE_GET_INT(i, &in); - if (i == 0 && strncmp(in, uri, i) != 0) { + if (i != uri_len || strncmp(in, uri, i) != 0) { unlink(fn); efree(buf); return NULL; @@ -3244,7 +3244,7 @@ sdlPtr get_sdl(zval *this_ptr, char *uri, zend_long cache_wsdl) } memcpy(key+len,md5str,sizeof(md5str)); - if ((sdl = get_sdl_from_cache(key, uri, t-SOAP_GLOBAL(cache_ttl), &cached)) != NULL) { + if ((sdl = get_sdl_from_cache(key, uri, uri_len, t-SOAP_GLOBAL(cache_ttl), &cached)) != NULL) { t = cached; efree(key); goto cache_in_memory; From deebb68612dc3c606845346993d938e09050be2d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 19 Oct 2023 15:07:20 +0200 Subject: [PATCH 3/3] Fix segfault and assertion failure with refcounted props and arrays Closes GH-12478. --- ext/soap/php_encoding.c | 4 +- .../tests/bugs/segfault_assertion_props.phpt | 51 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 ext/soap/tests/bugs/segfault_assertion_props.phpt diff --git a/ext/soap/php_encoding.c b/ext/soap/php_encoding.c index 3a4626aa5be..a5fbd3df9dd 100644 --- a/ext/soap/php_encoding.c +++ b/ext/soap/php_encoding.c @@ -1561,10 +1561,12 @@ static zval *to_zval_object_ex(zval *ret, encodeTypePtr type, xmlNodePtr data, z if (Z_TYPE_P(prop) != IS_ARRAY) { /* Convert into array */ array_init(&arr); - Z_ADDREF_P(prop); + Z_TRY_ADDREF_P(prop); add_next_index_zval(&arr, prop); set_zval_property(ret, (char*)trav->name, &arr); prop = &arr; + } else { + SEPARATE_ARRAY(prop); } /* Add array element */ add_next_index_zval(prop, &tmpVal); diff --git a/ext/soap/tests/bugs/segfault_assertion_props.phpt b/ext/soap/tests/bugs/segfault_assertion_props.phpt new file mode 100644 index 00000000000..9d496d72967 --- /dev/null +++ b/ext/soap/tests/bugs/segfault_assertion_props.phpt @@ -0,0 +1,51 @@ +--TEST-- +Segfault and assertion failure with refcounted props and arrays +--INI-- +soap.wsdl_cache_enabled=0 +--EXTENSIONS-- +soap +--FILE-- + + + + Hello + World + + +EOF; + } +} + +trait A { + public $a = [self::class . 'a']; + public $b = self::class . 'b'; +} + +class DummyClass { + use A; +} + +$client = new TestSoapClient(__DIR__."/../classmap.wsdl", ['classmap' => ['Struct' => 'DummyClass']]); +var_dump($client->dotest2("???")); +?> +--EXPECT-- +object(DummyClass)#2 (2) { + ["a"]=> + array(2) { + [0]=> + string(11) "DummyClassa" + [1]=> + string(5) "Hello" + } + ["b"]=> + array(2) { + [0]=> + string(11) "DummyClassb" + [1]=> + string(5) "World" + } +}