From 107bd080a5b02ad929e781c3e42bd8aa0ac79f9e Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 5 Feb 2025 14:13:56 +0100 Subject: [PATCH 1/3] Fix Clang style nits (GH-17685) This addresses all `-Wlogical-op-parentheses` and `-Wmissing-braces` warnings across the whole code base (all Windows specific code). --- Zend/zend_hrtime.h | 2 +- ext/opcache/zend_file_cache.c | 2 +- ext/openssl/xp_ssl.c | 2 +- ext/standard/basic_functions.c | 2 +- ext/standard/flock_compat.c | 3 +-- main/fopen_wrappers.c | 2 +- sapi/cli/php_cli.c | 4 ++-- win32/fnmatch.c | 4 ++-- win32/readdir.c | 2 +- win32/winutil.c | 2 +- 10 files changed, 12 insertions(+), 13 deletions(-) diff --git a/Zend/zend_hrtime.h b/Zend/zend_hrtime.h index 32ecb15fe33ac..994dd6da169ed 100644 --- a/Zend/zend_hrtime.h +++ b/Zend/zend_hrtime.h @@ -83,7 +83,7 @@ void zend_startup_hrtime(void); static zend_always_inline zend_hrtime_t zend_hrtime(void) { #if ZEND_HRTIME_PLATFORM_WINDOWS - LARGE_INTEGER lt = {0}; + LARGE_INTEGER lt = {{0}}; QueryPerformanceCounter(<); return (zend_hrtime_t)((zend_hrtime_t)lt.QuadPart * zend_hrtime_timer_scale); #elif ZEND_HRTIME_PLATFORM_APPLE_GETTIME_NSEC diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 74e0d19d1b6de..2a13f751ce3fa 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -75,7 +75,7 @@ # define LOCK_UN 2 static int zend_file_cache_flock(int fd, int op) { - OVERLAPPED offset = {0,0,0,0,NULL}; + OVERLAPPED offset = {0, 0, {{0}}, NULL}; if (op == LOCK_EX) { if (LockFileEx((HANDLE)_get_osfhandle(fd), LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, &offset) == TRUE) { diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 484abf483539d..92168c16175a1 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -758,7 +758,7 @@ static int php_openssl_win_cert_verify_callback(X509_STORE_CTX *x509_store_ctx, } { /* Then verify it against a policy */ - SSL_EXTRA_CERT_CHAIN_POLICY_PARA ssl_policy_params = {sizeof(SSL_EXTRA_CERT_CHAIN_POLICY_PARA)}; + SSL_EXTRA_CERT_CHAIN_POLICY_PARA ssl_policy_params = {{sizeof(SSL_EXTRA_CERT_CHAIN_POLICY_PARA)}}; CERT_CHAIN_POLICY_PARA chain_policy_params = {sizeof(CERT_CHAIN_POLICY_PARA)}; CERT_CHAIN_POLICY_STATUS chain_policy_status = {sizeof(CERT_CHAIN_POLICY_STATUS)}; BOOL verify_result; diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 4f9ed736b7134..1eca986b9cbb4 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -807,7 +807,7 @@ PHP_FUNCTION(putenv) valw = php_win32_cp_any_to_w(value); } /* valw may be NULL, but the failed conversion still needs to be checked. */ - if (!keyw || !valw && value) { + if (!keyw || (!valw && value)) { tsrm_env_unlock(); free(pe.putenv_string); zend_string_release(pe.key); diff --git a/ext/standard/flock_compat.c b/ext/standard/flock_compat.c index 50ad961f00492..b7e1df7cbc987 100644 --- a/ext/standard/flock_compat.c +++ b/ext/standard/flock_compat.c @@ -104,8 +104,7 @@ PHPAPI int php_flock(int fd, int operation) { HANDLE hdl = (HANDLE) _get_osfhandle(fd); DWORD low = 0xFFFFFFFF, high = 0xFFFFFFFF; - OVERLAPPED offset = - {0, 0, 0, 0, NULL}; + OVERLAPPED offset = {0, 0, {{0}}, NULL}; DWORD err; if (INVALID_HANDLE_VALUE == hdl) { diff --git a/main/fopen_wrappers.c b/main/fopen_wrappers.c index 99a082846b481..269863d73ecb9 100644 --- a/main/fopen_wrappers.c +++ b/main/fopen_wrappers.c @@ -531,7 +531,7 @@ PHPAPI zend_string *php_resolve_path(const char *filename, size_t filename_lengt IS_ABSOLUTE_PATH doesn't care about this path form till now. It might be a big thing to extend, thus just a local handling for now. */ - filename_length >=2 && IS_SLASH(filename[0]) && !IS_SLASH(filename[1]) || + (filename_length >=2 && IS_SLASH(filename[0]) && !IS_SLASH(filename[1])) || #endif !path || !*path) { diff --git a/sapi/cli/php_cli.c b/sapi/cli/php_cli.c index 941fab1c08d79..1f8a81e2d9755 100644 --- a/sapi/cli/php_cli.c +++ b/sapi/cli/php_cli.c @@ -840,9 +840,9 @@ static int do_cli(int argc, char **argv) /* {{{ */ is essential to mitigate buggy console info. */ interactive = php_win32_console_is_own() && !(script_file || - argc > php_optind && context.mode != PHP_CLI_MODE_CLI_DIRECT && + (argc > php_optind && context.mode != PHP_CLI_MODE_CLI_DIRECT && context.mode != PHP_CLI_MODE_PROCESS_STDIN && - strcmp(argv[php_optind-1],"--") + strcmp(argv[php_optind-1],"--")) ); } #endif diff --git a/win32/fnmatch.c b/win32/fnmatch.c index 296a1a5ca1843..a06931fd6ee79 100644 --- a/win32/fnmatch.c +++ b/win32/fnmatch.c @@ -137,8 +137,8 @@ PHPAPI int fnmatch(const char *pattern, const char *string, int flags) tolower((unsigned char)*string))) ; else if ((flags & FNM_PREFIX_DIRS) && *string == EOS && - (c == '/' && string != stringstart || - string == stringstart+1 && *stringstart == '/') ) + ((c == '/' && string != stringstart) || + (string == stringstart+1 && *stringstart == '/'))) return (0); else return (FNM_NOMATCH); diff --git a/win32/readdir.c b/win32/readdir.c index 89381e5834b62..804b6cb42d27b 100644 --- a/win32/readdir.c +++ b/win32/readdir.c @@ -59,7 +59,7 @@ DIR *opendir(const char *dir) wcscpy(filespecw, resolvedw); index = resolvedw_len - 1; } - if (index >= 0 && filespecw[index] == L'/' || index == 0 && filespecw[index] == L'\\') + if ((index >= 0 && filespecw[index] == L'/') || (index == 0 && filespecw[index] == L'\\')) filespecw[index] = L'\0'; wcscat(filespecw, L"\\*"); diff --git a/win32/winutil.c b/win32/winutil.c index 37b47c5a0f71d..cab4b5bac9b41 100644 --- a/win32/winutil.c +++ b/win32/winutil.c @@ -459,7 +459,7 @@ static zend_always_inline BOOL is_compatible(HMODULE handle, BOOL is_smaller, ch This check is to be extended as new VS versions come out. */ DWORD core_minor = (DWORD)(PHP_LINKER_MINOR/10); DWORD comp_minor = (DWORD)(minor/10); - if (14 == major && (is_smaller ? core_minor < comp_minor : core_minor > comp_minor) || PHP_LINKER_MAJOR != major) + if ((14 == major && (is_smaller ? core_minor < comp_minor : core_minor > comp_minor)) || PHP_LINKER_MAJOR != major) #else if (PHP_LINKER_MAJOR != major) #endif From e3798c2ab94d768e9f8753f4068e6612b118fa86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 5 Feb 2025 17:54:52 +0100 Subject: [PATCH 2/3] sapi/cli: Extend `--ini` to print INI settings changed from the builtin default (#17459) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * sapi/cli: Extend `--ini` to print INI settings changed from the builtin default This is intended to make it easier to check whether or not a given INI setting is changed from the default when building reproducers for a bugreport, without forgetting any that might be relevant to the report. As an example, running `sapi/cli/php -c /etc/php/8.3/cli/ --ini` on my Ubuntu will now output: Configuration File (php.ini) Path: /usr/local/lib Loaded Configuration File: /etc/php/8.3/cli/php.ini Scan for additional .ini files in: (none) Additional .ini files parsed: (none) Non-standard INI settings: allow_url_include: "0" -> "" auto_append_file: (none) -> "" auto_prepend_file: (none) -> "" display_errors: "1" -> "" display_startup_errors: "1" -> "" enable_dl: "1" -> "" error_reporting: (none) -> "22527" html_errors: "1" -> "0" ignore_repeated_errors: "0" -> "" ignore_repeated_source: "0" -> "" implicit_flush: "0" -> "1" log_errors: "0" -> "1" mail.add_x_header: "0" -> "" mail.mixed_lf_and_crlf: "0" -> "" max_execution_time: "30" -> "0" memory_limit: "128M" -> "-1" request_order: (none) -> "GP" session.cookie_httponly: "0" -> "" session.gc_divisor: "100" -> "1000" session.gc_probability: "1" -> "0" session.sid_bits_per_character: "4" -> "5" session.sid_length: "32" -> "26" short_open_tag: "1" -> "" unserialize_callback_func: (none) -> "" user_dir: (none) -> "" variables_order: "EGPCS" -> "GPCS" zend.assertions: "1" -> "-1" zend.exception_ignore_args: "0" -> "1" zend.exception_string_param_max_len: "15" -> "0" * Improve phrasing Co-authored-by: Michael Voříšek * NEWS/UPGRADING --------- Co-authored-by: Michael Voříšek --- NEWS | 4 ++++ UPGRADING | 1 + Zend/zend_ini.c | 1 + Zend/zend_ini.h | 1 + sapi/cli/php_cli.c | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 40 insertions(+) diff --git a/NEWS b/NEWS index 5148521e07900..5b21c028d3918 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.5.0alpha1 +- CLI: + . Extended --ini to print INI settings changed from the builtin default. + (timwolla) + - COM: . Fixed property access of PHP objects wrapped in variant. (cmb) . Fixed method calls for PHP objects wrapped in variant. (cmb) diff --git a/UPGRADING b/UPGRADING index c35d9477af899..7149579d4732a 100644 --- a/UPGRADING +++ b/UPGRADING @@ -107,6 +107,7 @@ PHP 8.5 UPGRADE NOTES - CLI: . Trying to set a process title that is too long with cli_set_process_title() will now fail instead of silently truncating the given title. + . --ini will now print INI settings changed from the builtin default. ======================================== 4. Deprecated Functionality diff --git a/Zend/zend_ini.c b/Zend/zend_ini.c index 6986481b5b312..199ebfb2e9b45 100644 --- a/Zend/zend_ini.c +++ b/Zend/zend_ini.c @@ -225,6 +225,7 @@ ZEND_API zend_result zend_register_ini_entries_ex(const zend_ini_entry_def *ini_ while (ini_entry->name) { p = pemalloc(sizeof(zend_ini_entry), 1); + p->def = ini_entry; p->name = zend_string_init_interned(ini_entry->name, ini_entry->name_length, 1); p->on_modify = ini_entry->on_modify; p->mh_arg1 = ini_entry->mh_arg1; diff --git a/Zend/zend_ini.h b/Zend/zend_ini.h index 1939d7b89e3ff..5a7377f1181d8 100644 --- a/Zend/zend_ini.h +++ b/Zend/zend_ini.h @@ -60,6 +60,7 @@ struct _zend_ini_entry { uint8_t orig_modifiable; uint8_t modified; + const zend_ini_entry_def *def; }; BEGIN_EXTERN_C() diff --git a/sapi/cli/php_cli.c b/sapi/cli/php_cli.c index 1f8a81e2d9755..b81055f84f271 100644 --- a/sapi/cli/php_cli.c +++ b/sapi/cli/php_cli.c @@ -588,6 +588,14 @@ BOOL WINAPI php_cli_win32_ctrl_handler(DWORD sig) #endif /*}}}*/ +static int zend_ini_entry_cmp(Bucket *a, Bucket *b) +{ + zend_ini_entry *A = Z_PTR(a->val); + zend_ini_entry *B = Z_PTR(b->val); + + return zend_binary_strcasecmp(ZSTR_VAL(A->name), ZSTR_LEN(A->name), ZSTR_VAL(B->name), ZSTR_LEN(B->name)); +} + static int do_cli(int argc, char **argv) /* {{{ */ { int c; @@ -1096,6 +1104,31 @@ static int do_cli(int argc, char **argv) /* {{{ */ zend_printf("Loaded Configuration File: %s\n", php_ini_opened_path ? php_ini_opened_path : "(none)"); zend_printf("Scan for additional .ini files in: %s\n", php_ini_scanned_path ? php_ini_scanned_path : "(none)"); zend_printf("Additional .ini files parsed: %s\n", php_ini_scanned_files ? php_ini_scanned_files : "(none)"); + zend_printf("\n"); + zend_printf("Non-default INI settings:\n"); + zend_ini_entry *ini_entry; + HashTable *sorted = zend_array_dup(EG(ini_directives)); + zend_array_sort(sorted, zend_ini_entry_cmp, 1); + ZEND_HASH_PACKED_FOREACH_PTR(sorted, ini_entry) { + if (ini_entry->value == NULL && ini_entry->def->value == NULL) { + continue; + } + if (ini_entry->value != NULL && ini_entry->def->value != NULL && zend_string_equals_cstr(ini_entry->value, ini_entry->def->value, ini_entry->def->value_length)) { + continue; + } + + zend_printf( + "%s: %s%s%s -> %s%s%s\n", + ZSTR_VAL(ini_entry->name), + ini_entry->def->value ? "\"" : "", + ini_entry->def->value ? ini_entry->def->value : "(none)", + ini_entry->def->value ? "\"" : "", + ini_entry->value ? "\"" : "", + ini_entry->value ? ZSTR_VAL(ini_entry->value) : "(none)", + ini_entry->value ? "\"" : "" + ); + } ZEND_HASH_FOREACH_END(); + zend_array_destroy(sorted); break; } } From ab6977d36c5b9066b27f8455e9dbda13d8c44964 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 29 Jan 2025 12:48:35 +0100 Subject: [PATCH 3/3] Fix segfault when assigning to backing value by-ref from hook Fixes oss-fuzz #391975641 Closes GH-17620 --- NEWS | 2 ++ Zend/tests/oss-fuzz-391975641.phpt | 22 ++++++++++++++++++++++ Zend/zend_execute.c | 2 +- 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/oss-fuzz-391975641.phpt diff --git a/NEWS b/NEWS index 7ccf92b9169c8..fb3a050b1bb21 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,8 @@ PHP NEWS . Fixed bug GH-17618 (UnhandledMatchError does not take zend.exception_ignore_args=1 into account). (timwolla) . Fix fallback paths in fast_long_{add,sub}_function. (nielsdos) + . Fixed bug OSS-Fuzz #391975641 (Crash when accessing property backing value + by reference). (ilutov) - DOM: . Fixed bug GH-17609 (Typo in error message: Dom\NO_DEFAULT_NS instead of diff --git a/Zend/tests/oss-fuzz-391975641.phpt b/Zend/tests/oss-fuzz-391975641.phpt new file mode 100644 index 0000000000000..586457aeac591 --- /dev/null +++ b/Zend/tests/oss-fuzz-391975641.phpt @@ -0,0 +1,22 @@ +--TEST-- +OSS-Fuzz #391975641: Segfault when creating reference from backing value +--FILE-- + $this->prop; + set { + $this->prop = &$value; + $value = &$this->prop; + } + } +} + +$c = new C; +$c->prop = 1; +var_dump($c->prop); + +?> +--EXPECT-- +int(1) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 8b148c40a4971..0f8b062e3f046 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -3490,7 +3490,7 @@ static zend_always_inline void zend_assign_to_property_reference(zval *container variable_ptr = zend_wrong_assign_to_variable_reference( variable_ptr, value_ptr, &garbage OPLINE_CC EXECUTE_DATA_CC); - } else if (prop_info) { + } else if (prop_info && ZEND_TYPE_IS_SET(prop_info->type)) { variable_ptr = zend_assign_to_typed_property_reference(prop_info, variable_ptr, value_ptr, &garbage EXECUTE_DATA_CC); } else { zend_assign_to_variable_reference(variable_ptr, value_ptr, &garbage);