From 0a6326c6ac93c53b8804d23d1d42bf91475a260d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 15 Apr 2025 23:58:33 +0200 Subject: [PATCH 1/3] Fix uouv when handling empty options in ZipArchive::addGlob() Reported by OpenAI AARDVARK. php_zip_parse_option is only called when options are passed to the function. Prior to this patch, php_zip_parse_option was responsible for zeroing the opts variable. So in the case when php_zip_parse_option is not called, opts remains uninitialized yet it is being used anyway. By just always zeroing opts at declaration time, we avoid this issue and we are unlikely to reintroduce this in the future. Closes GH-18329. --- NEWS | 3 +++ ext/zip/php_zip.c | 4 ++-- ext/zip/tests/addGlob_empty_options.phpt | 22 ++++++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 ext/zip/tests/addGlob_empty_options.phpt diff --git a/NEWS b/NEWS index 7d660e26cef75..a3f2990962d42 100644 --- a/NEWS +++ b/NEWS @@ -43,6 +43,9 @@ PHP NEWS leads to negative stream position). (David Carlier) . Fix resource leak in iptcembed() on error. (nielsdos) +- Zip: + . Fix uouv when handling empty options in ZipArchive::addGlob(). (nielsdos) + 10 Apr 2025, PHP 8.3.20 - Core: diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 172057685105b..be096ad56e29d 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -354,13 +354,13 @@ typedef struct { #endif } zip_options; +/* Expects opts to be zero-initialized. */ static int php_zip_parse_options(HashTable *options, zip_options *opts) /* {{{ */ { zval *option; /* default values */ - memset(opts, 0, sizeof(zip_options)); opts->flags = ZIP_FL_OVERWRITE; opts->comp_method = -1; /* -1 to not change default */ #ifdef HAVE_ENCRYPTION @@ -1732,7 +1732,7 @@ static void php_zip_add_from_pattern(INTERNAL_FUNCTION_PARAMETERS, int type) /* size_t path_len = 1; zend_long glob_flags = 0; HashTable *options = NULL; - zip_options opts; + zip_options opts = {0}; int found; zend_string *pattern; diff --git a/ext/zip/tests/addGlob_empty_options.phpt b/ext/zip/tests/addGlob_empty_options.phpt new file mode 100644 index 0000000000000..f4a4126059a7b --- /dev/null +++ b/ext/zip/tests/addGlob_empty_options.phpt @@ -0,0 +1,22 @@ +--TEST-- +addGlob with empty options +--EXTENSIONS-- +zip +--FILE-- +open($file, ZipArchive::CREATE | ZipArchive::OVERWRITE); +$zip->addGlob(__FILE__, 0, []); +var_dump($zip->statIndex(0)['name'] === __FILE__); +$zip->close(); + +?> +--CLEAN-- + +--EXPECT-- +bool(true) From 91c6c727d516716855d6fa8bd75908cd3249d17e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 15 Apr 2025 23:59:25 +0200 Subject: [PATCH 2/3] Fix memory leak when handling a too long path in ZipArchive::addGlob() Closes GH-18330. --- NEWS | 2 ++ ext/zip/php_zip.c | 3 +++ .../addGlob_too_long_add_path_option.phpt | 21 +++++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 ext/zip/tests/addGlob_too_long_add_path_option.phpt diff --git a/NEWS b/NEWS index a3f2990962d42..22ce2f45ecca0 100644 --- a/NEWS +++ b/NEWS @@ -45,6 +45,8 @@ PHP NEWS - Zip: . Fix uouv when handling empty options in ZipArchive::addGlob(). (nielsdos) + . Fix memory leak when handling a too long path in ZipArchive::addGlob(). + (nielsdos) 10 Apr 2025, PHP 8.3.20 diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index be096ad56e29d..45b83aeae63bc 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -1796,6 +1796,9 @@ static void php_zip_add_from_pattern(INTERNAL_FUNCTION_PARAMETERS, int type) /* if (opts.add_path) { if ((opts.add_path_len + file_stripped_len) > MAXPATHLEN) { + if (basename) { + zend_string_release_ex(basename, 0); + } php_error_docref(NULL, E_WARNING, "Entry name too long (max: %d, %zd given)", MAXPATHLEN - 1, (opts.add_path_len + file_stripped_len)); zend_array_destroy(Z_ARR_P(return_value)); diff --git a/ext/zip/tests/addGlob_too_long_add_path_option.phpt b/ext/zip/tests/addGlob_too_long_add_path_option.phpt new file mode 100644 index 0000000000000..9598eeca40a89 --- /dev/null +++ b/ext/zip/tests/addGlob_too_long_add_path_option.phpt @@ -0,0 +1,21 @@ +--TEST-- +addGlob with too long add_path option +--EXTENSIONS-- +zip +--FILE-- +open($file, ZipArchive::CREATE | ZipArchive::OVERWRITE); +$zip->addGlob(__FILE__, 0, ['add_path' => str_repeat('A', PHP_MAXPATHLEN - 2)]); +$zip->close(); + +?> +--CLEAN-- + +--EXPECTF-- +Warning: ZipArchive::addGlob(): Entry name too long (max: %d, %d given) in %s on line %d From c905d5910645b6024faee6ce791ab884e739c767 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 16 Apr 2025 00:09:35 +0200 Subject: [PATCH 3/3] Fix NULL deref on high modification key We should re-index in the loop. Closes GH-18331. --- NEWS | 1 + ext/ldap/ldap.c | 8 +++++--- ext/ldap/tests/ldap_modify_batch_error.phpt | 13 +++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 22ce2f45ecca0..ac31131ea9d22 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,7 @@ PHP NEWS - LDAP: . Fixed bug GH-17776 (LDAP_OPT_X_TLS_* options can't be overridden). (Remi) + . Fix NULL deref on high modification key. (nielsdos) - libxml: . Fixed custom external entity loader returning an invalid resource leading diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 1a878bfc58483..6c005337346b5 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2785,12 +2785,12 @@ PHP_FUNCTION(ldap_modify_batch) ldap_mods = safe_emalloc((num_mods+1), sizeof(LDAPMod *), 0); /* for each modification */ - for (i = 0; i < num_mods; i++) { + i = 0; + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(mods), fetched) { /* allocate the modification struct */ ldap_mods[i] = safe_emalloc(1, sizeof(LDAPMod), 0); /* fetch the relevant data */ - fetched = zend_hash_index_find(Z_ARRVAL_P(mods), i); mod = fetched; _ldap_hash_fetch(mod, LDAP_MODIFY_BATCH_ATTRIB, &attrib); @@ -2855,7 +2855,9 @@ PHP_FUNCTION(ldap_modify_batch) /* NULL-terminate values */ ldap_mods[i]->mod_bvalues[num_modvals] = NULL; } - } + + i++; + } ZEND_HASH_FOREACH_END(); /* NULL-terminate modifications */ ldap_mods[num_mods] = NULL; diff --git a/ext/ldap/tests/ldap_modify_batch_error.phpt b/ext/ldap/tests/ldap_modify_batch_error.phpt index bce62cafb2791..0ac093b4a0341 100644 --- a/ext/ldap/tests/ldap_modify_batch_error.phpt +++ b/ext/ldap/tests/ldap_modify_batch_error.phpt @@ -59,6 +59,16 @@ $mods = array( ) ); +var_dump(ldap_modify_batch($link, "dc=my-domain,$base", $mods)); + +// high key with invalid attribute type +$mods = [ + 99999 => [ + "attrib" => "weirdAttribute", + "modtype" => LDAP_MODIFY_BATCH_ADD, + "values" => ["value1"], + ], +]; var_dump(ldap_modify_batch($link, "dc=my-domain,$base", $mods)); ?> --CLEAN-- @@ -81,3 +91,6 @@ bool(false) Warning: ldap_modify_batch(): Batch Modify: Undefined attribute type in %s on line %d bool(false) + +Warning: ldap_modify_batch(): Batch Modify: Undefined attribute type in %s on line %d +bool(false)