From 6f5c9af1de83910f4e79a7711a5d5d7d42adee9b Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 30 Sep 2024 21:26:02 +0100 Subject: [PATCH 01/13] ext/ldap: Add tests for php_ldap_do_modify() And also amend some existing tests which would duplicate coverage --- ext/ldap/tests/ldap_add_error.phpt | 95 ++++------- ..._add_modify_delete_programming_errors.phpt | 156 ++++++++++++++++++ ..._delete_references_programming_errors.phpt | 87 ++++++++++ ext/ldap/tests/ldap_mod_add_error.phpt | 17 +- ext/ldap/tests/ldap_mod_del_error.phpt | 9 +- ext/ldap/tests/ldap_mod_replace_error.phpt | 9 +- ext/ldap/tests/ldap_modify_error.phpt | 17 +- 7 files changed, 300 insertions(+), 90 deletions(-) create mode 100644 ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt create mode 100644 ext/ldap/tests/ldap_add_modify_delete_references_programming_errors.phpt diff --git a/ext/ldap/tests/ldap_add_error.phpt b/ext/ldap/tests/ldap_add_error.phpt index d78276eca3e54..5ad0a7abc6e7a 100644 --- a/ext/ldap/tests/ldap_add_error.phpt +++ b/ext/ldap/tests/ldap_add_error.phpt @@ -13,73 +13,55 @@ require "connect.inc"; $link = ldap_connect_and_bind($uri, $user, $passwd, $protocol_version); -var_dump(ldap_add($link, "$base", array())); - // Invalid DN var_dump( - ldap_add($link, "weirdAttribute=val", array( - "weirdAttribute" => "val", - )), + ldap_add( + $link, + "weirdAttribute=val", + ["weirdAttribute" => "val"], + ), ldap_error($link), ldap_errno($link) ); // Duplicate entry -for ($i = 0; $i < 2; $i++) +for ($i = 0; $i < 2; $i++) { var_dump( - ldap_add($link, "dc=my-domain,$base", array( - "objectClass" => array( - "top", - "dcObject", - "organization"), - "dc" => "my-domain", - "o" => "my-domain", - )) + ldap_add( + $link, + "dc=my-domain,$base", + [ + "objectClass" => [ + "top", + "dcObject", + "organization", + ], + "dc" => "my-domain", + "o" => "my-domain", + ], + ) ); -var_dump(ldap_error($link), ldap_errno($link)); - -// Wrong array indexes -try { - ldap_add($link, "dc=my-domain2,dc=com", array( - "objectClass" => array( - 0 => "top", - 2 => "dcObject", - 5 => "organization"), - "dc" => "my-domain", - "o" => "my-domain", - )); - /* Is this correct behaviour to still have "Already exists" as error/errno? - , - ldap_error($link), - ldap_errno($link) - */ -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; } +var_dump(ldap_error($link), ldap_errno($link)); // Invalid attribute var_dump( - ldap_add($link, "$base", array( - "objectClass" => array( - "top", - "dcObject", - "organization"), - "dc" => "my-domain", - "o" => "my-domain", - "weirdAttr" => "weirdVal", - )), - ldap_error($link), - ldap_errno($link) -); - -var_dump( - ldap_add($link, "$base", array(array( "Oops" - ))) - /* Is this correct behaviour to still have "Undefined attribute type" as error/errno? - , + ldap_add( + $link, + "dc=my-domain,$base", + [ + "objectClass" => [ + "top", + "dcObject", + "organization", + ], + "dc" => "my-domain", + "o" => "my-domain", + "weirdAttr" => "weirdVal", + ], + ), ldap_error($link), - ldap_errno($link) - */ + ldap_errno($link), ); ?> --CLEAN-- @@ -91,9 +73,6 @@ $link = ldap_connect_and_bind($uri, $user, $passwd, $protocol_version); ldap_delete($link, "dc=my-domain,$base"); ?> --EXPECTF-- -Warning: ldap_add(): Add: Protocol error in %s on line %d -bool(false) - Warning: ldap_add(): Add: Invalid DN syntax in %s on line %d bool(false) string(17) "Invalid DN syntax" @@ -104,12 +83,8 @@ Warning: ldap_add(): Add: Already exists in %s on line %d bool(false) string(14) "Already exists" int(68) -ldap_add(): Argument #3 ($entry) must contain arrays with consecutive integer indices starting from 0 Warning: ldap_add(): Add: Undefined attribute type in %s on line %d bool(false) string(24) "Undefined attribute type" int(17) - -Warning: ldap_add(): Unknown attribute in the data in %s on line %d -bool(false) diff --git a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt new file mode 100644 index 0000000000000..f88b62a4841a7 --- /dev/null +++ b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt @@ -0,0 +1,156 @@ +--TEST-- +Programming errors (Value/Type errors) for ldap_add(_ext)(), ldap_mod_replace(_ext)(), ldap_mod_add(_ext)(), and ldap_mod_del(_ext)() +--EXTENSIONS-- +ldap +--FILE-- + 'value', + 'attribute2' => [ + 'value1', + 'value2', + ], +]; + +$empty_dict = []; +try { + var_dump(ldap_add($ldap, $valid_dn, $empty_dict)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$not_dict_of_attributes = [ + 'attribute1' => 'value', + 'not_key_entry', + 'attribute3' => [ + 'value1', + 'value2', + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $not_dict_of_attributes)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$dict_key_with_empty_key = [ + 'attribute1' => 'value', + '' => [ + 'value1', + 'value2', + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_with_empty_key)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$dict_key_with_nul_bytes = [ + "attrib1\0with\0nul\0byte" => 'value', + "attrib2\0with\0nul\0byte" => [ + 'value1', + 'value2', + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_with_nul_bytes)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$dict_key_value_not_string = [ + 'attribute1' => new stdClass(), + 'attribute2' => [ + 'value1', + 'value2', + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_value_not_string)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$dict_key_multi_value_empty_list = [ + 'attribute1' => 'value', + 'attribute2' => [], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_multi_value_empty_list)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$dict_key_multi_value_not_list = [ + 'attribute1' => 'value', + 'attribute2' => [ + 'value1', + 'wat' => 'nosense', + 'value2', + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_multi_value_not_list)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$dict_key_multi_value_not_list_of_strings = [ + 'attribute1' => 'value', + 'attribute2' => [ + 'value1', + [], + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_multi_value_not_list_of_strings)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$dict_key_multi_value_not_list_of_strings2 = [ + 'attribute1' => 'value', + 'attribute2' => [ + 'value1', + new stdClass(), + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_multi_value_not_list_of_strings2)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +/* We don't check that values have nul bytes as the length of the string is passed to LDAP */ + +?> +--EXPECTF-- +Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d +bool(false) + +Warning: ldap_add(): Unknown attribute in the data in %s on line %d +bool(false) + +Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d +bool(false) + +Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d +bool(false) +Error: Object of class stdClass could not be converted to string + +Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d +bool(false) +ValueError: ldap_add(): Argument #3 ($entry) must contain arrays with consecutive integer indices starting from 0 + +Warning: Array to string conversion in %s on line %d + +Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d +bool(false) +Error: Object of class stdClass could not be converted to string diff --git a/ext/ldap/tests/ldap_add_modify_delete_references_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_references_programming_errors.phpt new file mode 100644 index 0000000000000..b297ac70cc961 --- /dev/null +++ b/ext/ldap/tests/ldap_add_modify_delete_references_programming_errors.phpt @@ -0,0 +1,87 @@ +--TEST-- +Programming errors (Value/Type errors) for ldap_add(_ext)(), ldap_mod_replace(_ext)(), ldap_mod_add(_ext)(), and ldap_mod_del(_ext)() with references +--EXTENSIONS-- +ldap +--FILE-- + 'value', + 'attribute2' => [ + 'value1', + 'value2', + ], +]; + +$obj = new stdClass(); +$dict_key_value_not_string = [ + 'attribute1' => &$obj, + 'attribute2' => [ + 'value1', + 'value2', + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_value_not_string)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$empty_list = []; +$dict_key_multi_value_empty_list = [ + 'attribute1' => 'value', + 'attribute2' => &$empty_list, +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_multi_value_empty_list)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$empty_list = []; +$dict_key_multi_value_not_list_of_strings = [ + 'attribute1' => 'value', + 'attribute2' => [ + 'value1', + &$empty_list, + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_multi_value_not_list_of_strings)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$obj = new stdClass(); +$dict_key_multi_value_not_list_of_strings2 = [ + 'attribute1' => 'value', + 'attribute2' => [ + 'value1', + &$obj, + ], +]; +try { + var_dump(ldap_add($ldap, $valid_dn, $dict_key_multi_value_not_list_of_strings2)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +/* We don't check that values have nul bytes as the length of the string is passed to LDAP */ + +?> +--EXPECTF-- +Error: Object of class stdClass could not be converted to string + +Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d +bool(false) + +Warning: Array to string conversion in %s on line %d + +Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d +bool(false) +Error: Object of class stdClass could not be converted to string diff --git a/ext/ldap/tests/ldap_mod_add_error.phpt b/ext/ldap/tests/ldap_mod_add_error.phpt index 47aafb01612ad..40550acf388ad 100644 --- a/ext/ldap/tests/ldap_mod_add_error.phpt +++ b/ext/ldap/tests/ldap_mod_add_error.phpt @@ -14,19 +14,20 @@ require "connect.inc"; $link = ldap_connect_and_bind($uri, $user, $passwd, $protocol_version); // DN not found -var_dump(ldap_mod_add($link, "dc=my-domain,$base", array())); +var_dump(ldap_mod_add($link, "dc=my-domain,$base", ["dc" => "my-domain"])); // Invalid DN -var_dump(ldap_mod_add($link, "weirdAttribute=val", array())); +var_dump(ldap_mod_add($link, "weirdAttribute=val", ["dc" => "my-domain"])); -$entry = array( - "objectClass" => array( +$entry = [ + "objectClass" => [ "top", "dcObject", - "organization"), - "dc" => "my-domain", - "o" => "my-domain", -); + "organization", + ], + "dc" => "my-domain", + "o" => "my-domain", +]; ldap_add($link, "dc=my-domain,$base", $entry); diff --git a/ext/ldap/tests/ldap_mod_del_error.phpt b/ext/ldap/tests/ldap_mod_del_error.phpt index 2a6e81ba1492e..5c0650d9fe2e0 100644 --- a/ext/ldap/tests/ldap_mod_del_error.phpt +++ b/ext/ldap/tests/ldap_mod_del_error.phpt @@ -14,13 +14,11 @@ require "connect.inc"; $link = ldap_connect_and_bind($uri, $user, $passwd, $protocol_version); // DN not found -var_dump(ldap_mod_del($link, "dc=my-domain,$base", array())); +var_dump(ldap_mod_del($link, "dc=my-domain,$base", ["dc" => "my-domain"])); // Invalid DN -var_dump(ldap_mod_del($link, "weirdAttribute=val", array())); +var_dump(ldap_mod_del($link, "weirdAttribute=val", ["dc" => "my-domain"])); -// Invalid attributes -var_dump(ldap_mod_del($link, "$base", array('dc'))); ?> --CLEAN-- "my-domain"])); // Invalid DN -var_dump(ldap_mod_replace($link, "weirdAttribute=val", array())); +var_dump(ldap_mod_replace($link, "weirdAttribute=val", ["dc" => "my-domain"])); -// Invalid attributes -var_dump(ldap_mod_replace($link, "$base", array('dc'))); ?> --CLEAN-- "my-domain"])); // Invalid DN -var_dump(ldap_modify($link, "weirdAttribute=val", array())); +var_dump(ldap_modify($link, "weirdAttribute=val", ["dc" => "my-domain"])); -$entry = array( - "objectClass" => array( +$entry = [ + "objectClass" => [ "top", "dcObject", - "organization"), - "dc" => "my-domain", - "o" => "my-domain", -); + "organization", + ], + "dc" => "my-domain", + "o" => "my-domain", +]; ldap_add($link, "dc=my-domain,$base", $entry); From 2bac60246a16328f4a44bbcfb61af2ecc8ea3405 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 30 Sep 2024 22:40:27 +0100 Subject: [PATCH 02/13] ext/ldap: php_ldap_do_modify() throw ValueError when entries array is empty --- ext/ldap/ldap.c | 5 +++++ .../tests/ldap_add_modify_delete_programming_errors.phpt | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 82d4975a76913..6a67a968e9510 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2216,6 +2216,11 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) VERIFY_LDAP_LINK_CONNECTED(ld); num_attribs = zend_hash_num_elements(Z_ARRVAL_P(entry)); + if (num_attribs == 0) { + zend_argument_must_not_be_empty_error(3); + RETURN_THROWS(); + } + ldap_mods = safe_emalloc((num_attribs+1), sizeof(LDAPMod *), 0); num_berval = safe_emalloc(num_attribs, sizeof(int), 0); zend_hash_internal_pointer_reset(Z_ARRVAL_P(entry)); diff --git a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt index f88b62a4841a7..9886159c3bc22 100644 --- a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt +++ b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt @@ -132,8 +132,7 @@ try { ?> --EXPECTF-- -Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d -bool(false) +ValueError: ldap_add(): Argument #3 ($entry) must not be empty Warning: ldap_add(): Unknown attribute in the data in %s on line %d bool(false) From c76c93450265ae968187601c4232733f23b3a131 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 30 Sep 2024 22:51:02 +0100 Subject: [PATCH 03/13] ext/ldap: Handle attribute => value case directly --- ext/ldap/ldap.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 6a67a968e9510..d3f738884e126 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2251,29 +2251,30 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) value = zend_hash_get_current_data(Z_ARRVAL_P(entry)); ZVAL_DEREF(value); + /* If the attribute takes a single value it can be passed directly instead of as a list with one element */ + /* allow for arrays with one element, no allowance for arrays with none but probably not required, gerrit thomson. */ if (Z_TYPE_P(value) != IS_ARRAY) { - num_values = 1; - } else { - SEPARATE_ARRAY(value); - num_values = zend_hash_num_elements(Z_ARRVAL_P(value)); - } - - num_berval[i] = num_values; - ldap_mods[i]->mod_bvalues = safe_emalloc((num_values + 1), sizeof(struct berval *), 0); - -/* allow for arrays with one element, no allowance for arrays with none but probably not required, gerrit thomson. */ - if ((num_values == 1) && (Z_TYPE_P(value) != IS_ARRAY)) { convert_to_string(value); if (EG(exception)) { RETVAL_FALSE; num_berval[i] = 0; num_attribs = i + 1; + ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } + num_berval[i] = 1; + ldap_mods[i]->mod_bvalues = safe_emalloc(2, sizeof(struct berval *), 0); ldap_mods[i]->mod_bvalues[0] = (struct berval *) emalloc (sizeof(struct berval)); ldap_mods[i]->mod_bvalues[0]->bv_val = Z_STRVAL_P(value); ldap_mods[i]->mod_bvalues[0]->bv_len = Z_STRLEN_P(value); + ldap_mods[i]->mod_bvalues[1] = NULL; } else { + SEPARATE_ARRAY(value); + num_values = zend_hash_num_elements(Z_ARRVAL_P(value)); + + num_berval[i] = num_values; + ldap_mods[i]->mod_bvalues = safe_emalloc((num_values + 1), sizeof(struct berval *), 0); + for (j = 0; j < num_values; j++) { if ((ivalue = zend_hash_index_find(Z_ARRVAL_P(value), j)) == NULL) { zend_argument_value_error(3, "must contain arrays with consecutive integer indices starting from 0"); @@ -2293,8 +2294,9 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) ldap_mods[i]->mod_bvalues[j]->bv_val = Z_STRVAL_P(ivalue); ldap_mods[i]->mod_bvalues[j]->bv_len = Z_STRLEN_P(ivalue); } + ldap_mods[i]->mod_bvalues[num_values] = NULL; } - ldap_mods[i]->mod_bvalues[num_values] = NULL; + zend_hash_move_forward(Z_ARRVAL_P(entry)); } ldap_mods[num_attribs] = NULL; From 888018b4f57b053e0cddfbc020970c075f5393c2 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 30 Sep 2024 23:23:04 +0100 Subject: [PATCH 04/13] ext/ldap: Ensure list of attribute values is not empty --- ext/ldap/ldap.c | 8 ++++++++ .../tests/ldap_add_modify_delete_programming_errors.phpt | 4 +--- ...p_add_modify_delete_references_programming_errors.phpt | 4 +--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index d3f738884e126..982b075dc17c9 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2271,6 +2271,14 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) } else { SEPARATE_ARRAY(value); num_values = zend_hash_num_elements(Z_ARRVAL_P(value)); + if (num_values == 0) { + zend_argument_value_error(3, "list of attribute values must not be empty"); + RETVAL_FALSE; + num_berval[i] = 0; + num_attribs = i + 1; + ldap_mods[i]->mod_bvalues = NULL; + goto cleanup; + } num_berval[i] = num_values; ldap_mods[i]->mod_bvalues = safe_emalloc((num_values + 1), sizeof(struct berval *), 0); diff --git a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt index 9886159c3bc22..645a3e92c1ad6 100644 --- a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt +++ b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt @@ -143,9 +143,7 @@ bool(false) Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d bool(false) Error: Object of class stdClass could not be converted to string - -Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d -bool(false) +ValueError: ldap_add(): Argument #3 ($entry) list of attribute values must not be empty ValueError: ldap_add(): Argument #3 ($entry) must contain arrays with consecutive integer indices starting from 0 Warning: Array to string conversion in %s on line %d diff --git a/ext/ldap/tests/ldap_add_modify_delete_references_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_references_programming_errors.phpt index b297ac70cc961..e3249081d0e3f 100644 --- a/ext/ldap/tests/ldap_add_modify_delete_references_programming_errors.phpt +++ b/ext/ldap/tests/ldap_add_modify_delete_references_programming_errors.phpt @@ -76,9 +76,7 @@ try { ?> --EXPECTF-- Error: Object of class stdClass could not be converted to string - -Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d -bool(false) +ValueError: ldap_add(): Argument #3 ($entry) list of attribute values must not be empty Warning: Array to string conversion in %s on line %d From 891a47b4c39b0e3528167457e94c0207f29363e9 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 30 Sep 2024 23:25:32 +0100 Subject: [PATCH 05/13] ext/ldap: Check that attribute values is a list before traversal --- ext/ldap/ldap.c | 19 +++++++++++-------- ..._add_modify_delete_programming_errors.phpt | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 982b075dc17c9..fc89bd77dea1c 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2194,7 +2194,7 @@ PHP_FUNCTION(ldap_dn2ufn) static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) { zval *serverctrls = NULL; - zval *link, *entry, *value, *ivalue; + zval *link, *entry, *value; ldap_linkdata *ld; char *dn; LDAPMod **ldap_mods; @@ -2279,18 +2279,21 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } + if (!zend_array_is_list(Z_ARRVAL_P(value))) { + zend_argument_value_error(3, "must be a list of attribute values"); + RETVAL_FALSE; + num_berval[i] = 0; + num_attribs = i + 1; + ldap_mods[i]->mod_bvalues = NULL; + goto cleanup; + } num_berval[i] = num_values; ldap_mods[i]->mod_bvalues = safe_emalloc((num_values + 1), sizeof(struct berval *), 0); for (j = 0; j < num_values; j++) { - if ((ivalue = zend_hash_index_find(Z_ARRVAL_P(value), j)) == NULL) { - zend_argument_value_error(3, "must contain arrays with consecutive integer indices starting from 0"); - num_berval[i] = j; - num_attribs = i + 1; - RETVAL_FALSE; - goto cleanup; - } + zval *ivalue = zend_hash_index_find(Z_ARRVAL_P(value), j); + ZEND_ASSERT(ivalue != NULL); convert_to_string(ivalue); if (EG(exception)) { num_berval[i] = j; diff --git a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt index 645a3e92c1ad6..1be5fec755610 100644 --- a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt +++ b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt @@ -144,7 +144,7 @@ Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d bool(false) Error: Object of class stdClass could not be converted to string ValueError: ldap_add(): Argument #3 ($entry) list of attribute values must not be empty -ValueError: ldap_add(): Argument #3 ($entry) must contain arrays with consecutive integer indices starting from 0 +ValueError: ldap_add(): Argument #3 ($entry) must be a list of attribute values Warning: Array to string conversion in %s on line %d From 037880b4feb7aaa184dae6c3efba54df79d50d28 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 30 Sep 2024 23:35:13 +0100 Subject: [PATCH 06/13] ext/ldap: Reduce scope of variables --- ext/ldap/ldap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index fc89bd77dea1c..83df935fe6042 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2194,14 +2194,14 @@ PHP_FUNCTION(ldap_dn2ufn) static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) { zval *serverctrls = NULL; - zval *link, *entry, *value; + zval *link, *entry; ldap_linkdata *ld; char *dn; LDAPMod **ldap_mods; LDAPControl **lserverctrls = NULL; ldap_resultdata *result; LDAPMessage *ldap_res; - int i, j, num_attribs, num_values, msgid; + int i, j, num_attribs, msgid; size_t dn_len; int *num_berval; zend_string *attribute; @@ -2248,7 +2248,7 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) goto cleanup; } - value = zend_hash_get_current_data(Z_ARRVAL_P(entry)); + zval *value = zend_hash_get_current_data(Z_ARRVAL_P(entry)); ZVAL_DEREF(value); /* If the attribute takes a single value it can be passed directly instead of as a list with one element */ @@ -2270,7 +2270,7 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) ldap_mods[i]->mod_bvalues[1] = NULL; } else { SEPARATE_ARRAY(value); - num_values = zend_hash_num_elements(Z_ARRVAL_P(value)); + int num_values = zend_hash_num_elements(Z_ARRVAL_P(value)); if (num_values == 0) { zend_argument_value_error(3, "list of attribute values must not be empty"); RETVAL_FALSE; From d149cf83325f3ffecb4d0c72f7deff384ff514bd Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 30 Sep 2024 23:37:54 +0100 Subject: [PATCH 07/13] ext/ldap: Use HASH_FOREACH macro --- ext/ldap/ldap.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 83df935fe6042..08818be726733 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2291,20 +2291,20 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) num_berval[i] = num_values; ldap_mods[i]->mod_bvalues = safe_emalloc((num_values + 1), sizeof(struct berval *), 0); - for (j = 0; j < num_values; j++) { - zval *ivalue = zend_hash_index_find(Z_ARRVAL_P(value), j); - ZEND_ASSERT(ivalue != NULL); - convert_to_string(ivalue); + zend_ulong attribute_value_index = 0; + zval *attribute_value = NULL; + ZEND_HASH_FOREACH_NUM_KEY_VAL(Z_ARRVAL_P(value), attribute_value_index, attribute_value) { + convert_to_string(attribute_value); if (EG(exception)) { - num_berval[i] = j; + num_berval[i] = (int)attribute_value_index; num_attribs = i + 1; RETVAL_FALSE; goto cleanup; } - ldap_mods[i]->mod_bvalues[j] = (struct berval *) emalloc (sizeof(struct berval)); - ldap_mods[i]->mod_bvalues[j]->bv_val = Z_STRVAL_P(ivalue); - ldap_mods[i]->mod_bvalues[j]->bv_len = Z_STRLEN_P(ivalue); - } + ldap_mods[i]->mod_bvalues[attribute_value_index] = (struct berval *) emalloc (sizeof(struct berval)); + ldap_mods[i]->mod_bvalues[attribute_value_index]->bv_val = Z_STRVAL_P(attribute_value); + ldap_mods[i]->mod_bvalues[attribute_value_index]->bv_len = Z_STRLEN_P(attribute_value); + } ZEND_HASH_FOREACH_END(); ldap_mods[i]->mod_bvalues[num_values] = NULL; } From fb0051f6a9b4e5e011665ea9a6ba9a5797d99976 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Oct 2024 00:00:10 +0100 Subject: [PATCH 08/13] ext/ldap: Zero out arrays and traverse them as NULL terminated list --- ext/ldap/ldap.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 08818be726733..299e855975d43 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2201,9 +2201,8 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) LDAPControl **lserverctrls = NULL; ldap_resultdata *result; LDAPMessage *ldap_res; - int i, j, num_attribs, msgid; + int i, num_attribs, msgid; size_t dn_len; - int *num_berval; zend_string *attribute; zend_ulong index; int is_full_add=0; /* flag for full add operation so ldap_mod_add can be put back into oper, gerrit THomson */ @@ -2222,7 +2221,8 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) } ldap_mods = safe_emalloc((num_attribs+1), sizeof(LDAPMod *), 0); - num_berval = safe_emalloc(num_attribs, sizeof(int), 0); + /* Zero out the list */ + memset(ldap_mods, 0, sizeof(LDAPMod *) * (num_attribs+1)); zend_hash_internal_pointer_reset(Z_ARRVAL_P(entry)); /* added by gerrit thomson to fix ldap_add using ldap_mod_add */ @@ -2242,8 +2242,6 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) } else { php_error_docref(NULL, E_WARNING, "Unknown attribute in the data"); RETVAL_FALSE; - num_berval[i] = 0; - num_attribs = i + 1; ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } @@ -2257,12 +2255,9 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) convert_to_string(value); if (EG(exception)) { RETVAL_FALSE; - num_berval[i] = 0; - num_attribs = i + 1; ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } - num_berval[i] = 1; ldap_mods[i]->mod_bvalues = safe_emalloc(2, sizeof(struct berval *), 0); ldap_mods[i]->mod_bvalues[0] = (struct berval *) emalloc (sizeof(struct berval)); ldap_mods[i]->mod_bvalues[0]->bv_val = Z_STRVAL_P(value); @@ -2274,30 +2269,25 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) if (num_values == 0) { zend_argument_value_error(3, "list of attribute values must not be empty"); RETVAL_FALSE; - num_berval[i] = 0; - num_attribs = i + 1; ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } if (!zend_array_is_list(Z_ARRVAL_P(value))) { zend_argument_value_error(3, "must be a list of attribute values"); RETVAL_FALSE; - num_berval[i] = 0; - num_attribs = i + 1; ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } - num_berval[i] = num_values; ldap_mods[i]->mod_bvalues = safe_emalloc((num_values + 1), sizeof(struct berval *), 0); + /* Zero out the list */ + memset(ldap_mods[i]->mod_bvalues, 0, sizeof(struct berval *) * (num_values+1)); zend_ulong attribute_value_index = 0; zval *attribute_value = NULL; ZEND_HASH_FOREACH_NUM_KEY_VAL(Z_ARRVAL_P(value), attribute_value_index, attribute_value) { convert_to_string(attribute_value); if (EG(exception)) { - num_berval[i] = (int)attribute_value_index; - num_attribs = i + 1; RETVAL_FALSE; goto cleanup; } @@ -2368,15 +2358,19 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) } cleanup: - for (i = 0; i < num_attribs; i++) { - efree(ldap_mods[i]->mod_type); - for (j = 0; j < num_berval[i]; j++) { - efree(ldap_mods[i]->mod_bvalues[j]); + for (LDAPMod **ptr = ldap_mods; *ptr != NULL; ptr++) { + LDAPMod *mod = *ptr; + if (mod->mod_type) { + efree(mod->mod_type); + } + if (mod->mod_bvalues != NULL) { + for (struct berval **bval_ptr = mod->mod_bvalues; *bval_ptr != NULL; bval_ptr++) { + efree(*bval_ptr); + } + efree(mod->mod_bvalues); } - efree(ldap_mods[i]->mod_bvalues); - efree(ldap_mods[i]); + efree(mod); } - efree(num_berval); efree(ldap_mods); if (lserverctrls) { From 6d449010576ad5a329ef6fd690fcb0eea1c64f65 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Oct 2024 00:16:03 +0100 Subject: [PATCH 09/13] ext/ldap: Refactor loop to a HASH_FOREACH loop --- ext/ldap/ldap.c | 78 ++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 43 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 299e855975d43..b2b8650d50406 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2194,27 +2194,26 @@ PHP_FUNCTION(ldap_dn2ufn) static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) { zval *serverctrls = NULL; - zval *link, *entry; + zval *link; ldap_linkdata *ld; char *dn; + HashTable *attributes_ht; LDAPMod **ldap_mods; LDAPControl **lserverctrls = NULL; ldap_resultdata *result; LDAPMessage *ldap_res; - int i, num_attribs, msgid; + int i, msgid; size_t dn_len; - zend_string *attribute; - zend_ulong index; int is_full_add=0; /* flag for full add operation so ldap_mod_add can be put back into oper, gerrit THomson */ - if (zend_parse_parameters(ZEND_NUM_ARGS(), "Opa/|a!", &link, ldap_link_ce, &dn, &dn_len, &entry, &serverctrls) != SUCCESS) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oph/|a!", &link, ldap_link_ce, &dn, &dn_len, &attributes_ht, &serverctrls) != SUCCESS) { RETURN_THROWS(); } ld = Z_LDAP_LINK_P(link); VERIFY_LDAP_LINK_CONNECTED(ld); - num_attribs = zend_hash_num_elements(Z_ARRVAL_P(entry)); + uint32_t num_attribs = zend_hash_num_elements(attributes_ht); if (num_attribs == 0) { zend_argument_must_not_be_empty_error(3); RETURN_THROWS(); @@ -2223,7 +2222,6 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) ldap_mods = safe_emalloc((num_attribs+1), sizeof(LDAPMod *), 0); /* Zero out the list */ memset(ldap_mods, 0, sizeof(LDAPMod *) * (num_attribs+1)); - zend_hash_internal_pointer_reset(Z_ARRVAL_P(entry)); /* added by gerrit thomson to fix ldap_add using ldap_mod_add */ if (oper == PHP_LD_FULL_ADD) { @@ -2232,74 +2230,70 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) } /* end additional , gerrit thomson */ - for (i = 0; i < num_attribs; i++) { - ldap_mods[i] = emalloc(sizeof(LDAPMod)); - ldap_mods[i]->mod_op = oper | LDAP_MOD_BVALUES; - ldap_mods[i]->mod_type = NULL; - - if (zend_hash_get_current_key(Z_ARRVAL_P(entry), &attribute, &index) == HASH_KEY_IS_STRING) { - ldap_mods[i]->mod_type = estrndup(ZSTR_VAL(attribute), ZSTR_LEN(attribute)); - } else { + const zend_string *attribute = NULL; + zval *attribute_values = NULL; + unsigned int attribute_index = 0; + ZEND_HASH_FOREACH_STR_KEY_VAL(attributes_ht, attribute, attribute_values) { + if (attribute == NULL) { php_error_docref(NULL, E_WARNING, "Unknown attribute in the data"); RETVAL_FALSE; - ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } - zval *value = zend_hash_get_current_data(Z_ARRVAL_P(entry)); + ldap_mods[attribute_index] = emalloc(sizeof(LDAPMod)); + ldap_mods[attribute_index]->mod_op = oper | LDAP_MOD_BVALUES; + ldap_mods[attribute_index]->mod_type = estrndup(ZSTR_VAL(attribute), ZSTR_LEN(attribute)); + ldap_mods[attribute_index]->mod_bvalues = NULL; - ZVAL_DEREF(value); + ZVAL_DEREF(attribute_values); /* If the attribute takes a single value it can be passed directly instead of as a list with one element */ /* allow for arrays with one element, no allowance for arrays with none but probably not required, gerrit thomson. */ - if (Z_TYPE_P(value) != IS_ARRAY) { - convert_to_string(value); + if (Z_TYPE_P(attribute_values) != IS_ARRAY) { + convert_to_string(attribute_values); if (EG(exception)) { RETVAL_FALSE; - ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } - ldap_mods[i]->mod_bvalues = safe_emalloc(2, sizeof(struct berval *), 0); - ldap_mods[i]->mod_bvalues[0] = (struct berval *) emalloc (sizeof(struct berval)); - ldap_mods[i]->mod_bvalues[0]->bv_val = Z_STRVAL_P(value); - ldap_mods[i]->mod_bvalues[0]->bv_len = Z_STRLEN_P(value); - ldap_mods[i]->mod_bvalues[1] = NULL; + ldap_mods[attribute_index]->mod_bvalues = safe_emalloc(2, sizeof(struct berval *), 0); + ldap_mods[attribute_index]->mod_bvalues[0] = (struct berval *) emalloc (sizeof(struct berval)); + ldap_mods[attribute_index]->mod_bvalues[0]->bv_val = Z_STRVAL_P(attribute_values); + ldap_mods[attribute_index]->mod_bvalues[0]->bv_len = Z_STRLEN_P(attribute_values); + ldap_mods[attribute_index]->mod_bvalues[1] = NULL; } else { - SEPARATE_ARRAY(value); - int num_values = zend_hash_num_elements(Z_ARRVAL_P(value)); + SEPARATE_ARRAY(attribute_values); + uint32_t num_values = zend_hash_num_elements(Z_ARRVAL_P(attribute_values)); if (num_values == 0) { zend_argument_value_error(3, "list of attribute values must not be empty"); RETVAL_FALSE; - ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } - if (!zend_array_is_list(Z_ARRVAL_P(value))) { + if (!zend_array_is_list(Z_ARRVAL_P(attribute_values))) { zend_argument_value_error(3, "must be a list of attribute values"); RETVAL_FALSE; - ldap_mods[i]->mod_bvalues = NULL; goto cleanup; } - ldap_mods[i]->mod_bvalues = safe_emalloc((num_values + 1), sizeof(struct berval *), 0); + ldap_mods[attribute_index]->mod_bvalues = safe_emalloc((num_values + 1), sizeof(struct berval *), 0); /* Zero out the list */ - memset(ldap_mods[i]->mod_bvalues, 0, sizeof(struct berval *) * (num_values+1)); + memset(ldap_mods[attribute_index]->mod_bvalues, 0, sizeof(struct berval *) * (num_values+1)); zend_ulong attribute_value_index = 0; zval *attribute_value = NULL; - ZEND_HASH_FOREACH_NUM_KEY_VAL(Z_ARRVAL_P(value), attribute_value_index, attribute_value) { + ZEND_HASH_FOREACH_NUM_KEY_VAL(Z_ARRVAL_P(attribute_values), attribute_value_index, attribute_value) { convert_to_string(attribute_value); if (EG(exception)) { RETVAL_FALSE; goto cleanup; } - ldap_mods[i]->mod_bvalues[attribute_value_index] = (struct berval *) emalloc (sizeof(struct berval)); - ldap_mods[i]->mod_bvalues[attribute_value_index]->bv_val = Z_STRVAL_P(attribute_value); - ldap_mods[i]->mod_bvalues[attribute_value_index]->bv_len = Z_STRLEN_P(attribute_value); + ldap_mods[attribute_index]->mod_bvalues[attribute_value_index] = (struct berval *) emalloc (sizeof(struct berval)); + ldap_mods[attribute_index]->mod_bvalues[attribute_value_index]->bv_val = Z_STRVAL_P(attribute_value); + ldap_mods[attribute_index]->mod_bvalues[attribute_value_index]->bv_len = Z_STRLEN_P(attribute_value); } ZEND_HASH_FOREACH_END(); - ldap_mods[i]->mod_bvalues[num_values] = NULL; + ldap_mods[attribute_index]->mod_bvalues[num_values] = NULL; } - zend_hash_move_forward(Z_ARRVAL_P(entry)); - } + attribute_index++; + } ZEND_HASH_FOREACH_END(); ldap_mods[num_attribs] = NULL; if (serverctrls) { @@ -2360,9 +2354,7 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) cleanup: for (LDAPMod **ptr = ldap_mods; *ptr != NULL; ptr++) { LDAPMod *mod = *ptr; - if (mod->mod_type) { - efree(mod->mod_type); - } + efree(mod->mod_type); if (mod->mod_bvalues != NULL) { for (struct berval **bval_ptr = mod->mod_bvalues; *bval_ptr != NULL; bval_ptr++) { efree(*bval_ptr); From b3c07435099b2acd84a2621fd8f7715265a56315 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Oct 2024 00:20:39 +0100 Subject: [PATCH 10/13] ext/ldap: Promote warning to ValueError if array is not a dict --- ext/ldap/ldap.c | 2 +- ext/ldap/tests/gh16136.phpt | 5 ++--- .../tests/ldap_add_modify_delete_programming_errors.phpt | 4 +--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index b2b8650d50406..52ce567097c91 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2235,7 +2235,7 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) unsigned int attribute_index = 0; ZEND_HASH_FOREACH_STR_KEY_VAL(attributes_ht, attribute, attribute_values) { if (attribute == NULL) { - php_error_docref(NULL, E_WARNING, "Unknown attribute in the data"); + zend_argument_value_error(3, "must be an associative array of attribute => values"); RETVAL_FALSE; goto cleanup; } diff --git a/ext/ldap/tests/gh16136.phpt b/ext/ldap/tests/gh16136.phpt index 14f21e3e757f2..9283402158c14 100644 --- a/ext/ldap/tests/gh16136.phpt +++ b/ext/ldap/tests/gh16136.phpt @@ -25,6 +25,5 @@ try { } ?> ---EXPECTF-- -Warning: ldap_add(): Unknown attribute in the data in %s on line %d -bool(false) +--EXPECT-- +ValueError: ldap_add(): Argument #3 ($entry) must be an associative array of attribute => values diff --git a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt index 1be5fec755610..8840922607271 100644 --- a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt +++ b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt @@ -133,9 +133,7 @@ try { ?> --EXPECTF-- ValueError: ldap_add(): Argument #3 ($entry) must not be empty - -Warning: ldap_add(): Unknown attribute in the data in %s on line %d -bool(false) +ValueError: ldap_add(): Argument #3 ($entry) must be an associative array of attribute => values Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d bool(false) From be1f7c2815a503d252d76501430d3b6c7357245e Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Oct 2024 00:24:06 +0100 Subject: [PATCH 11/13] ext/ldap: Check array key does not have any nul bytes --- ext/ldap/ldap.c | 5 +++++ .../tests/ldap_add_modify_delete_programming_errors.phpt | 4 +--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 52ce567097c91..14eff31de2328 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2239,6 +2239,11 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) RETVAL_FALSE; goto cleanup; } + if (zend_str_has_nul_byte(attribute)) { + zend_argument_value_error(3, "key must not contain any null bytes"); + RETVAL_FALSE; + goto cleanup; + } ldap_mods[attribute_index] = emalloc(sizeof(LDAPMod)); ldap_mods[attribute_index]->mod_op = oper | LDAP_MOD_BVALUES; diff --git a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt index 8840922607271..1d0d21e06dedd 100644 --- a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt +++ b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt @@ -137,9 +137,7 @@ ValueError: ldap_add(): Argument #3 ($entry) must be an associative array of att Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d bool(false) - -Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d -bool(false) +ValueError: ldap_add(): Argument #3 ($entry) key must not contain any null bytes Error: Object of class stdClass could not be converted to string ValueError: ldap_add(): Argument #3 ($entry) list of attribute values must not be empty ValueError: ldap_add(): Argument #3 ($entry) must be a list of attribute values From ff25d5bea575c42ae5f030a7ca73d2df453446ba Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Oct 2024 00:28:25 +0100 Subject: [PATCH 12/13] ext/ldap: Check that array key is not empty --- ext/ldap/ldap.c | 5 +++++ .../tests/ldap_add_modify_delete_programming_errors.phpt | 4 +--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 14eff31de2328..feb3935e24b52 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2239,6 +2239,11 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) RETVAL_FALSE; goto cleanup; } + if (ZSTR_LEN(attribute) == 0) { + zend_argument_value_error(3, "key must not be empty"); + RETVAL_FALSE; + goto cleanup; + } if (zend_str_has_nul_byte(attribute)) { zend_argument_value_error(3, "key must not contain any null bytes"); RETVAL_FALSE; diff --git a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt index 1d0d21e06dedd..ae3ec0be52e84 100644 --- a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt +++ b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt @@ -134,9 +134,7 @@ try { --EXPECTF-- ValueError: ldap_add(): Argument #3 ($entry) must not be empty ValueError: ldap_add(): Argument #3 ($entry) must be an associative array of attribute => values - -Warning: ldap_add(): Add: Can't contact LDAP server in %s on line %d -bool(false) +ValueError: ldap_add(): Argument #3 ($entry) key must not be empty ValueError: ldap_add(): Argument #3 ($entry) key must not contain any null bytes Error: Object of class stdClass could not be converted to string ValueError: ldap_add(): Argument #3 ($entry) list of attribute values must not be empty From 0ef85bb5525ebc36e05227121f53c88496d73ce4 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 1 Oct 2024 00:35:02 +0100 Subject: [PATCH 13/13] ext/ldap: Rename variable and move closer to usage site --- ext/ldap/ldap.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index feb3935e24b52..47ebcb244e3bb 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2202,7 +2202,6 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) LDAPControl **lserverctrls = NULL; ldap_resultdata *result; LDAPMessage *ldap_res; - int i, msgid; size_t dn_len; int is_full_add=0; /* flag for full add operation so ldap_mod_add can be put back into oper, gerrit THomson */ @@ -2314,19 +2313,21 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) } } -/* check flag to see if do_mod was called to perform full add , gerrit thomson */ + /* check flag to see if do_mod was called to perform full add , gerrit thomson */ + int ldap_status_code = LDAP_SUCCESS; + int msgid; if (is_full_add == 1) { if (ext) { - i = ldap_add_ext(ld->link, dn, ldap_mods, lserverctrls, NULL, &msgid); + ldap_status_code = ldap_add_ext(ld->link, dn, ldap_mods, lserverctrls, NULL, &msgid); } else { - i = ldap_add_ext_s(ld->link, dn, ldap_mods, lserverctrls, NULL); + ldap_status_code = ldap_add_ext_s(ld->link, dn, ldap_mods, lserverctrls, NULL); } - if (i != LDAP_SUCCESS) { - php_error_docref(NULL, E_WARNING, "Add: %s", ldap_err2string(i)); + if (ldap_status_code != LDAP_SUCCESS) { + php_error_docref(NULL, E_WARNING, "Add: %s", ldap_err2string(ldap_status_code)); RETVAL_FALSE; } else if (ext) { - i = ldap_result(ld->link, msgid, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res); - if (i == -1) { + ldap_status_code = ldap_result(ld->link, msgid, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res); + if (ldap_status_code == -1) { php_error_docref(NULL, E_WARNING, "Add operation failed"); RETVAL_FALSE; goto cleanup; @@ -2339,16 +2340,16 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) } else RETVAL_TRUE; } else { if (ext) { - i = ldap_modify_ext(ld->link, dn, ldap_mods, lserverctrls, NULL, &msgid); + ldap_status_code = ldap_modify_ext(ld->link, dn, ldap_mods, lserverctrls, NULL, &msgid); } else { - i = ldap_modify_ext_s(ld->link, dn, ldap_mods, lserverctrls, NULL); + ldap_status_code = ldap_modify_ext_s(ld->link, dn, ldap_mods, lserverctrls, NULL); } - if (i != LDAP_SUCCESS) { - php_error_docref(NULL, E_WARNING, "Modify: %s", ldap_err2string(i)); + if (ldap_status_code != LDAP_SUCCESS) { + php_error_docref(NULL, E_WARNING, "Modify: %s", ldap_err2string(ldap_status_code)); RETVAL_FALSE; } else if (ext) { - i = ldap_result(ld->link, msgid, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res); - if (i == -1) { + ldap_status_code = ldap_result(ld->link, msgid, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res); + if (ldap_status_code == -1) { php_error_docref(NULL, E_WARNING, "Modify operation failed"); RETVAL_FALSE; goto cleanup;