From e36f517584649ca48978814ab3bb75c5dd08136b Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 25 Sep 2024 16:52:22 +0100 Subject: [PATCH 1/2] ext/ldap: Fix GH-16032 (Various NULL pointer dereferencements in ldap_modify_batch()) --- ext/ldap/ldap.c | 14 ++++++++++++++ ext/ldap/tests/gh16032-1.phpt | 26 ++++++++++++++++++++++++++ ext/ldap/tests/gh16032-2.phpt | 26 ++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) create mode 100644 ext/ldap/tests/gh16032-1.phpt create mode 100644 ext/ldap/tests/gh16032-2.phpt diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 51eca3d7cc4de..bf02527b2b89a 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2546,6 +2546,9 @@ PHP_FUNCTION(ldap_modify_batch) num_modprops = zend_hash_num_elements(Z_ARRVAL_P(mod)); for (j = 0; j < num_modprops; j++) { + bool has_attrib_key = false; + bool has_modtype_key = false; + /* are the keys strings? */ if (zend_hash_get_current_key(Z_ARRVAL_P(mod), &modkey, &tmpUlong) != HASH_KEY_IS_STRING) { zend_argument_type_error(3, "must only contain string-indexed arrays"); @@ -2567,6 +2570,7 @@ PHP_FUNCTION(ldap_modify_batch) /* does the value type match the key? */ if (_ldap_str_equal_to_const(ZSTR_VAL(modkey), ZSTR_LEN(modkey), LDAP_MODIFY_BATCH_ATTRIB)) { + has_attrib_key = true; if (Z_TYPE_P(modinfo) != IS_STRING) { zend_type_error("%s(): Option \"" LDAP_MODIFY_BATCH_ATTRIB "\" must be of type string, %s given", get_active_function_name(), zend_zval_type_name(modinfo)); RETURN_THROWS(); @@ -2578,6 +2582,7 @@ PHP_FUNCTION(ldap_modify_batch) } } else if (_ldap_str_equal_to_const(ZSTR_VAL(modkey), ZSTR_LEN(modkey), LDAP_MODIFY_BATCH_MODTYPE)) { + has_modtype_key = true; if (Z_TYPE_P(modinfo) != IS_LONG) { zend_type_error("%s(): Option \"" LDAP_MODIFY_BATCH_MODTYPE "\" must be of type int, %s given", get_active_function_name(), zend_zval_type_name(modinfo)); RETURN_THROWS(); @@ -2639,6 +2644,15 @@ PHP_FUNCTION(ldap_modify_batch) } } + if (!has_attrib_key) { + zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_ATTRIB "\" is missing", get_active_function_name()); + RETURN_THROWS(); + } + if (!has_modtype_key) { + zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_MODTYPE "\" is missing", get_active_function_name()); + RETURN_THROWS(); + } + zend_hash_move_forward(Z_ARRVAL_P(mod)); } } diff --git a/ext/ldap/tests/gh16032-1.phpt b/ext/ldap/tests/gh16032-1.phpt new file mode 100644 index 0000000000000..dbaf8213933d7 --- /dev/null +++ b/ext/ldap/tests/gh16032-1.phpt @@ -0,0 +1,26 @@ +--TEST-- +Bug GH-16032: Various NULL pointer dereferencements in ldap_modify_batch() +--EXTENSIONS-- +ldap +--FILE-- + LDAP_MODIFY_BATCH_ADD, + "values" => ["value1"], + ], +]; +try { + var_dump(ldap_modify_batch($ldap, $valid_dn, $modification_missing_attrib_key)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +ValueError: ldap_modify_batch(): Required option "attrib" is missing diff --git a/ext/ldap/tests/gh16032-2.phpt b/ext/ldap/tests/gh16032-2.phpt new file mode 100644 index 0000000000000..531415807f674 --- /dev/null +++ b/ext/ldap/tests/gh16032-2.phpt @@ -0,0 +1,26 @@ +--TEST-- +Bug GH-16032: Various NULL pointer dereferencements in ldap_modify_batch() +--EXTENSIONS-- +ldap +--FILE-- + "attrib1", + "values" => ["value1"], + ], +]; +try { + var_dump(ldap_modify_batch($ldap, $valid_dn, $modification_missing_modtype_key)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +ValueError: ldap_modify_batch(): Required option "modtype" is missing From ba0d5b4c2786d9e886dd587b8569507158479584 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 25 Sep 2024 18:12:17 +0100 Subject: [PATCH 2/2] The conditions need to be outside the inner loop --- ext/ldap/ldap.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index bf02527b2b89a..ae65f0258c24b 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2544,10 +2544,10 @@ PHP_FUNCTION(ldap_modify_batch) /* for the modification hashtable... */ zend_hash_internal_pointer_reset(Z_ARRVAL_P(mod)); num_modprops = zend_hash_num_elements(Z_ARRVAL_P(mod)); + bool has_attrib_key = false; + bool has_modtype_key = false; for (j = 0; j < num_modprops; j++) { - bool has_attrib_key = false; - bool has_modtype_key = false; /* are the keys strings? */ if (zend_hash_get_current_key(Z_ARRVAL_P(mod), &modkey, &tmpUlong) != HASH_KEY_IS_STRING) { @@ -2644,17 +2644,17 @@ PHP_FUNCTION(ldap_modify_batch) } } - if (!has_attrib_key) { - zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_ATTRIB "\" is missing", get_active_function_name()); - RETURN_THROWS(); - } - if (!has_modtype_key) { - zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_MODTYPE "\" is missing", get_active_function_name()); - RETURN_THROWS(); - } - zend_hash_move_forward(Z_ARRVAL_P(mod)); } + + if (!has_attrib_key) { + zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_ATTRIB "\" is missing", get_active_function_name()); + RETURN_THROWS(); + } + if (!has_modtype_key) { + zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_MODTYPE "\" is missing", get_active_function_name()); + RETURN_THROWS(); + } } } /* validation was successful */