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] 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); }