From b87ca2f0f319d0f5444f6dd4bd6ef3772e17fe68 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 2 Oct 2024 13:32:58 +0100 Subject: [PATCH 1/5] ext/ldap: Add API parsing zval to LDAP value --- ext/ldap/ldap.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 53035d1f6023b..05447aadd90e8 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -234,6 +234,26 @@ static void ldap_result_entry_free_obj(zend_object *obj) } \ } +/* An LDAP value must be a string, however it defines a format for integer and + * booleans, thus we parse zvals to the corresponding string if possible + * See RFC 4517: https://datatracker.ietf.org/doc/html/rfc4517 */ +static zend_string* php_ldap_try_get_ldap_value_from_zval(zval *zv) { + switch (Z_TYPE_P(zv)) { + case IS_STRING: + case IS_LONG: + /* Object might be stringable */ + case IS_OBJECT: + return zval_try_get_string(zv); + case IS_TRUE: + return ZSTR_INIT_LITERAL("TRUE", false); + case IS_FALSE: + return ZSTR_INIT_LITERAL("FALSE", false); + default: + zend_type_error("LDAP value must be of type string|int|bool, %s given", zend_zval_value_name(zv)); + return NULL; + } +} + /* {{{ Parse controls from and to arrays */ static void _php_ldap_control_to_array(LDAP *ld, LDAPControl* ctrl, zval* array, int request) { From 9bf85b4bce25b8858d916860eb74dbf3651f4c4b Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 2 Oct 2024 13:42:58 +0100 Subject: [PATCH 2/5] ext/ldap: Parse attribute value via new API in do_modify Add a new API to free a zend_string via its char* --- ext/ldap/ldap.c | 27 ++++++++++++------- ..._add_modify_delete_programming_errors.phpt | 8 ++---- ..._delete_references_programming_errors.phpt | 10 +++---- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 05447aadd90e8..e4c763439a5b3 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -254,6 +254,11 @@ static zend_string* php_ldap_try_get_ldap_value_from_zval(zval *zv) { } } +/* The char pointer MUST refer to the char* of a zend_string struct */ +static void php_ldap_zend_string_release_from_char_pointer(char *ptr) { + zend_string_release((zend_string*) (ptr - XtOffsetOf(zend_string, val))); +} + /* {{{ Parse controls from and to arrays */ static void _php_ldap_control_to_array(LDAP *ld, LDAPControl* ctrl, zval* array, int request) { @@ -2278,15 +2283,16 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) /* 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(attribute_values) != IS_ARRAY) { - convert_to_string(attribute_values); - if (EG(exception)) { + zend_string *value = php_ldap_try_get_ldap_value_from_zval(attribute_values); + if (UNEXPECTED(value == NULL)) { RETVAL_FALSE; goto cleanup; } 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); + /* The string will be free by php_ldap_zend_string_release_from_char_pointer() during cleanup */ + ldap_mods[attribute_index]->mod_bvalues[0]->bv_val = ZSTR_VAL(value); + ldap_mods[attribute_index]->mod_bvalues[0]->bv_len = ZSTR_LEN(value); ldap_mods[attribute_index]->mod_bvalues[1] = NULL; } else { SEPARATE_ARRAY(attribute_values); @@ -2309,14 +2315,15 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) zend_ulong attribute_value_index = 0; zval *attribute_value = NULL; ZEND_HASH_FOREACH_NUM_KEY_VAL(Z_ARRVAL_P(attribute_values), attribute_value_index, attribute_value) { - convert_to_string(attribute_value); - if (EG(exception)) { + zend_string *value = php_ldap_try_get_ldap_value_from_zval(attribute_value); + if (UNEXPECTED(value == NULL)) { RETVAL_FALSE; goto cleanup; } 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); + /* The string will be free by php_ldap_zend_string_release_from_char_pointer() during cleanup */ + ldap_mods[attribute_index]->mod_bvalues[attribute_value_index]->bv_val = ZSTR_VAL(value); + ldap_mods[attribute_index]->mod_bvalues[attribute_value_index]->bv_len = ZSTR_LEN(value); } ZEND_HASH_FOREACH_END(); ldap_mods[attribute_index]->mod_bvalues[num_values] = NULL; } @@ -2388,7 +2395,9 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) 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); + struct berval *bval = *bval_ptr; + php_ldap_zend_string_release_from_char_pointer(bval->bv_val); + efree(bval); } efree(mod->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 ae3ec0be52e84..14fa50312e7d1 100644 --- a/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt +++ b/ext/ldap/tests/ldap_add_modify_delete_programming_errors.phpt @@ -131,7 +131,7 @@ try { /* We don't check that values have nul bytes as the length of the string is passed to LDAP */ ?> ---EXPECTF-- +--EXPECT-- ValueError: ldap_add(): Argument #3 ($entry) must not be empty ValueError: ldap_add(): Argument #3 ($entry) must be an associative array of attribute => values ValueError: ldap_add(): Argument #3 ($entry) key must not be empty @@ -139,9 +139,5 @@ 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 - -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) +TypeError: LDAP value must be of type string|int|bool, array given 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 index e3249081d0e3f..4926dbb841d0c 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 @@ -74,12 +74,8 @@ try { /* We don't check that values have nul bytes as the length of the string is passed to LDAP */ ?> ---EXPECTF-- +--EXPECT-- 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 - -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 +TypeError: LDAP value must be of type string|int|bool, array given +TypeError: LDAP value must be of type string|int|bool, stdClass given From cdfcb5e26e89ee6b4042b01adfb71000652c2cb5 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 2 Oct 2024 23:51:51 +0100 Subject: [PATCH 3/5] ext/ldap: Remove an unnecessary duplication --- ext/ldap/ldap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index e4c763439a5b3..0f06145ca1944 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2254,7 +2254,7 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) } /* end additional , gerrit thomson */ - const zend_string *attribute = NULL; + zend_string *attribute = NULL; zval *attribute_values = NULL; unsigned int attribute_index = 0; ZEND_HASH_FOREACH_STR_KEY_VAL(attributes_ht, attribute, attribute_values) { @@ -2276,7 +2276,8 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext) 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)); + /* No need to duplicate the string as it is not consumed and the zend_string will not be released */ + ldap_mods[attribute_index]->mod_type = ZSTR_VAL(attribute); ldap_mods[attribute_index]->mod_bvalues = NULL; ZVAL_DEREF(attribute_values); @@ -2392,7 +2393,6 @@ 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; - efree(mod->mod_type); if (mod->mod_bvalues != NULL) { for (struct berval **bval_ptr = mod->mod_bvalues; *bval_ptr != NULL; bval_ptr++) { struct berval *bval = *bval_ptr; From 2fb35ed3bd62ea2ae9ddfb74844f3fedb790d73c Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 3 Oct 2024 00:08:26 +0100 Subject: [PATCH 4/5] ext/ldap: Use bool instead of int --- ext/ldap/ldap.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 0f06145ca1944..a9335fdbbd14a 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -410,7 +410,6 @@ static int _php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, zval* arra { zval* val; zend_string *control_oid; - int control_iscritical = 0, rc = LDAP_SUCCESS; char** ldap_attrs = NULL; LDAPSortKey** sort_keys = NULL; zend_string *tmpstring = NULL, **tmpstrings1 = NULL, **tmpstrings2 = NULL; @@ -426,15 +425,15 @@ static int _php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, zval* arra return -1; } + bool control_iscritical = false; if ((val = zend_hash_str_find(Z_ARRVAL_P(array), "iscritical", sizeof("iscritical") - 1)) != NULL) { control_iscritical = zend_is_true(val); - } else { - control_iscritical = 0; } BerElement *ber = NULL; struct berval control_value = { 0L, NULL }; - int control_value_alloc = 0; + bool control_value_alloc = false; + int rc = LDAP_SUCCESS; if ((val = zend_hash_find(Z_ARRVAL_P(array), ZSTR_KNOWN(ZEND_STR_VALUE))) != NULL) { if (Z_TYPE_P(val) != IS_ARRAY) { From 0968aa420454ffc22bad6c40e57ab949957278f0 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 3 Oct 2024 00:17:23 +0100 Subject: [PATCH 5/5] ext/ldap: Pass a HashTable directly to parse individual control Rename function to not have a leading "_" at the same time --- ext/ldap/ldap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index a9335fdbbd14a..6f5830e54413d 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -406,7 +406,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, zval* array) +static int php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, const HashTable *control_ht) { zval* val; zend_string *control_oid; @@ -415,7 +415,7 @@ static int _php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, zval* arra zend_string *tmpstring = NULL, **tmpstrings1 = NULL, **tmpstrings2 = NULL; size_t num_tmpstrings1 = 0, num_tmpstrings2 = 0; - if ((val = zend_hash_str_find(Z_ARRVAL_P(array), "oid", sizeof("oid") - 1)) == NULL) { + if ((val = zend_hash_str_find(control_ht, "oid", sizeof("oid") - 1)) == NULL) { zend_value_error("%s(): Control must have an \"oid\" key", get_active_function_name()); return -1; } @@ -426,7 +426,7 @@ static int _php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, zval* arra } bool control_iscritical = false; - if ((val = zend_hash_str_find(Z_ARRVAL_P(array), "iscritical", sizeof("iscritical") - 1)) != NULL) { + if ((val = zend_hash_str_find(control_ht, "iscritical", sizeof("iscritical") - 1)) != NULL) { control_iscritical = zend_is_true(val); } @@ -435,7 +435,7 @@ static int _php_ldap_control_from_array(LDAP *ld, LDAPControl** ctrl, zval* arra bool control_value_alloc = false; int rc = LDAP_SUCCESS; - if ((val = zend_hash_find(Z_ARRVAL_P(array), ZSTR_KNOWN(ZEND_STR_VALUE))) != NULL) { + 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)) { @@ -786,7 +786,7 @@ static LDAPControl** php_ldap_controls_from_array(LDAP *ld, const HashTable *con break; } - if (_php_ldap_control_from_array(ld, ctrlp, ctrlarray) == LDAP_SUCCESS) { + if (php_ldap_control_from_array(ld, ctrlp, Z_ARRVAL_P(ctrlarray)) == LDAP_SUCCESS) { ++ctrlp; } else { error = 1;