From 8908a81973076967c53f0818b893dd18295ebffc Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Mon, 8 Dec 2025 13:28:03 -0800 Subject: [PATCH 1/3] ldap: Use cheaper checks for getting a string (#20652) Avoids loading globals, the return register can be used directly. --- ext/ldap/ldap.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index e9acd1899066..c2b08d0c6705 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -425,7 +425,7 @@ static void _php_ldap_control_to_array(LDAP *ld, LDAPControl* ctrl, zval* array, static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashTable *control_ht) { zval* val; - zend_string *control_oid; + zend_string *control_oid, *control_oid_tmp; char** ldap_attrs = NULL; LDAPSortKey** sort_keys = NULL; zend_string *tmpstring = NULL, **tmpstrings1 = NULL, **tmpstrings2 = NULL; @@ -436,8 +436,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT return -1; } - control_oid = zval_get_string(val); - if (EG(exception)) { + control_oid = zval_try_get_tmp_string(val, &control_oid_tmp); + if (!control_oid) { return -1; } @@ -453,8 +453,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT if ((val = zend_hash_find(control_ht, ZSTR_KNOWN(ZEND_STR_VALUE))) != NULL) { if (Z_TYPE_P(val) != IS_ARRAY) { - tmpstring = zval_get_string(val); - if (EG(exception)) { + tmpstring = zval_try_get_string(val); + if (!tmpstring) { rc = -1; goto failure; } @@ -468,8 +468,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT pagesize = zval_get_long(tmp); } if ((tmp = zend_hash_str_find(Z_ARRVAL_P(val), "cookie", sizeof("cookie") - 1)) != NULL) { - tmpstring = zval_get_string(tmp); - if (EG(exception)) { + tmpstring = zval_try_get_string(tmp); + if (!tmpstring) { rc = -1; goto failure; } @@ -488,8 +488,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT rc = -1; zend_value_error("%s(): Control must have a \"filter\" key", get_active_function_name()); } else { - zend_string* assert = zval_get_string(tmp); - if (EG(exception)) { + zend_string* assert = zval_try_get_string(tmp); + if (!assert) { rc = -1; goto failure; } @@ -516,8 +516,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT rc = -1; php_error_docref(NULL, E_WARNING, "Failed to allocate control value"); } else { - tmpstring = zval_get_string(tmp); - if (EG(exception)) { + tmpstring = zval_try_get_string(tmp); + if (!tmpstring) { rc = -1; goto failure; } @@ -555,8 +555,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT goto failure; } - tmpstrings1[num_tmpstrings1] = zval_get_string(attr); - if (EG(exception)) { + tmpstrings1[num_tmpstrings1] = zval_try_get_string(attr); + if (!tmpstrings1[num_tmpstrings1]) { rc = -1; goto failure; } @@ -603,8 +603,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT goto failure; } sort_keys[i] = emalloc(sizeof(LDAPSortKey)); - tmpstrings1[num_tmpstrings1] = zval_get_string(tmp); - if (EG(exception)) { + tmpstrings1[num_tmpstrings1] = zval_try_get_string(tmp); + if (!tmpstrings1[num_tmpstrings1]) { rc = -1; goto failure; } @@ -612,8 +612,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT ++num_tmpstrings1; if ((tmp = zend_hash_str_find(Z_ARRVAL_P(sortkey), "oid", sizeof("oid") - 1)) != NULL) { - tmpstrings2[num_tmpstrings2] = zval_get_string(tmp); - if (EG(exception)) { + tmpstrings2[num_tmpstrings2] = zval_try_get_string(tmp); + if (!tmpstrings2[num_tmpstrings2]) { rc = -1; goto failure; } @@ -659,8 +659,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT } if ((tmp = zend_hash_str_find(Z_ARRVAL_P(val), "attrvalue", sizeof("attrvalue") - 1)) != NULL) { - tmpstring = zval_get_string(tmp); - if (EG(exception)) { + tmpstring = zval_try_get_string(tmp); + if (!tmpstring) { rc = -1; goto failure; } @@ -685,8 +685,8 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT } if ((tmp = zend_hash_str_find(Z_ARRVAL_P(val), "context", sizeof("context") - 1)) != NULL) { - tmpstring = zval_get_string(tmp); - if (EG(exception)) { + tmpstring = zval_try_get_string(tmp); + if (!tmpstring) { rc = -1; goto failure; } @@ -714,7 +714,7 @@ static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashT } failure: - zend_string_release(control_oid); + zend_tmp_string_release(control_oid_tmp); if (tmpstring != NULL) { zend_string_release(tmpstring); } @@ -2791,8 +2791,8 @@ PHP_FUNCTION(ldap_modify_batch) zend_ulong value_index = 0; zval *modification_value_zv = NULL; ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(modification_values), modification_value_zv) { - zend_string *modval = zval_get_string(modification_value_zv); - if (EG(exception)) { + zend_string *modval = zval_try_get_string(modification_value_zv); + if (!modval) { RETVAL_FALSE; ldap_mods[modification_index]->mod_bvalues[value_index] = NULL; num_mods = modification_index + 1; From 02a7c49564bebb0af476db7a37ad14b658119691 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Sat, 6 Dec 2025 14:09:01 +0100 Subject: [PATCH 2/3] ldap: Fix memory leak in ldap_set_options() Closes GH-20659. --- NEWS | 2 ++ ext/ldap/ldap.c | 10 ++++-- ...dap_set_option_leak_attrvalue_context.phpt | 34 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 ext/ldap/tests/ldap_set_option_leak_attrvalue_context.phpt diff --git a/NEWS b/NEWS index b4a15b3ddd8f..be1c55a7c8e8 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,8 @@ PHP NEWS - GD: . Fixed bug GH-20622 (imagestring/imagestringup overflow). (David Carlier) +- LDAP: + . Fix memory leak in ldap_set_options(). (ndossche) 18 Dec 2025, PHP 8.3.29 diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 0b0a0c21df4a..303ba055458e 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -664,14 +664,15 @@ static int _php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, zval* arra goto failure; } + zend_string *context_str = NULL; if ((tmp = zend_hash_str_find(Z_ARRVAL_P(val), "context", sizeof("context") - 1)) != NULL) { - tmpstring = zval_get_string(tmp); + context_str = zval_get_string(tmp); if (EG(exception)) { rc = -1; goto failure; } - context.bv_val = ZSTR_VAL(tmpstring); - context.bv_len = ZSTR_LEN(tmpstring); + context.bv_val = ZSTR_VAL(context_str); + context.bv_len = ZSTR_LEN(context_str); vlvInfo.ldvlv_context = &context; } else { vlvInfo.ldvlv_context = NULL; @@ -683,6 +684,9 @@ static int _php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, zval* arra if (rc != LDAP_SUCCESS) { php_error_docref(NULL, E_WARNING, "Failed to create VLV control value: %s (%d)", ldap_err2string(rc), rc); } + if (context_str) { + zend_string_release_ex(context_str, false); + } } else { zend_type_error("%s(): Control OID %s cannot be of type array", get_active_function_name(), ZSTR_VAL(control_oid)); rc = -1; diff --git a/ext/ldap/tests/ldap_set_option_leak_attrvalue_context.phpt b/ext/ldap/tests/ldap_set_option_leak_attrvalue_context.phpt new file mode 100644 index 000000000000..5829ad24faf5 --- /dev/null +++ b/ext/ldap/tests/ldap_set_option_leak_attrvalue_context.phpt @@ -0,0 +1,34 @@ +--TEST-- +ldap_set_option() - Leaks attrvalue and context +--EXTENSIONS-- +ldap +--FILE-- + "2.16.840.1.113730.3.4.9", "value" => ["attrvalue" => $attrvalue, "context" => $context, "before" => 0, "after" => 0]], +]; + +ldap_set_option($link, LDAP_OPT_CLIENT_CONTROLS, $controls); +ldap_get_option($link, LDAP_OPT_CLIENT_CONTROLS, $controls_out); + +var_dump($controls_out); +?> +--EXPECTF-- +array(1) { + ["2.16.840.1.113730.3.4.9"]=> + array(3) { + ["oid"]=> + string(23) "2.16.840.1.113730.3.4.9" + ["iscritical"]=> + bool(false) + ["value"]=> + string(28) "0%0%0 attrvaluecontext" + } +} From cb413b5d5f96ad97a8cc2d4f0ad0b065051f122f Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Thu, 4 Dec 2025 14:31:29 +0100 Subject: [PATCH 3/3] Update clang in macOS build This resolves a crash in release builds. This may be dropped again in the future once the bugfix lands. Co-authored by Alexandre Daubois Co-authored by Arnaud Le Blanc Co-authored by Jakub Zelenka Closes GH-20669 --- .github/actions/macos-update-clang/action.yml | 17 +++++++++++++++++ .github/workflows/nightly.yml | 2 ++ .github/workflows/push.yml | 2 ++ 3 files changed, 21 insertions(+) create mode 100644 .github/actions/macos-update-clang/action.yml diff --git a/.github/actions/macos-update-clang/action.yml b/.github/actions/macos-update-clang/action.yml new file mode 100644 index 000000000000..bceb431aed5a --- /dev/null +++ b/.github/actions/macos-update-clang/action.yml @@ -0,0 +1,17 @@ +name: Update clang +runs: + using: composite + steps: + - shell: bash + run: | + softwareupdate -l + label=$((softwareupdate -l 2>/dev/null | grep 'Label:' | grep -o 'Command Line Tools for Xcode.*' | head -1) || echo '') + if [ -n "$label" ]; then + softwareupdate -i "$label" + xcode_path=$(ls -1 '/Applications' | grep 'Xcode_.*\.app' | head -1) + sudo xcode-select -s "/Applications/$xcode_path" + else + echo "Not found." + fi + which clang + clang -v diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 909a412e796e..e67deb8ffe4f 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -367,6 +367,8 @@ jobs: uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} + - name: Update clang + uses: ./.github/actions/macos-update-clang - name: brew uses: ./.github/actions/brew - name: ./configure diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 028255330d1f..054f4a15286d 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -117,6 +117,8 @@ jobs: steps: - name: git checkout uses: actions/checkout@v5 + - name: Update clang + uses: ./.github/actions/macos-update-clang - name: brew uses: ./.github/actions/brew - name: ccache