Skip to content

Commit

Permalink
Backport fix GH-17280: ldap_search() fails when $attributes array has…
Browse files Browse the repository at this point in the history
… holes

Backport of GH-17284 to fix GH-17280 on lower branches.
  • Loading branch information
nielsdos committed Dec 27, 2024
1 parent b621b3a commit c05268a
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 43 deletions.
95 changes: 56 additions & 39 deletions ext/ldap/ldap.c
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,22 @@ static void ldap_result_entry_free_obj(zend_object *obj)
} \
}

static bool php_ldap_is_numerically_indexed_array(zend_array *arr)
{
if (zend_hash_num_elements(arr) == 0 || HT_IS_PACKED(arr)) {
return true;
}

zend_string *str_key;
ZEND_HASH_MAP_FOREACH_STR_KEY(arr, str_key) {
if (str_key) {
return false;
}
} ZEND_HASH_FOREACH_END();

return false;
}

/* {{{ Parse controls from and to arrays */
static void _php_ldap_control_to_array(LDAP *ld, LDAPControl* ctrl, zval* array, int request)
{
Expand Down Expand Up @@ -1471,20 +1487,22 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
num_attribs = zend_hash_num_elements(Z_ARRVAL_P(attrs));
ldap_attrs = safe_emalloc((num_attribs+1), sizeof(char *), 0);

for (i = 0; i<num_attribs; i++) {
if ((attr = zend_hash_index_find(Z_ARRVAL_P(attrs), i)) == NULL) {
php_error_docref(NULL, E_WARNING, "Array initialization wrong");
ret = 0;
goto cleanup;
}
if (!php_ldap_is_numerically_indexed_array(Z_ARRVAL_P(attrs))) {
php_error_docref(NULL, E_WARNING, "Argument #4 ($attributes) must be an array with numeric keys");
ret = 0;
goto cleanup;
}

i = 0;
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(attrs), attr) {
convert_to_string(attr);
if (EG(exception)) {
ret = 0;
goto cleanup;
}
ldap_attrs[i] = Z_STRVAL_P(attr);
}
ldap_attrs[i++] = Z_STRVAL_P(attr);
} ZEND_HASH_FOREACH_END();

ldap_attrs[num_attribs] = NULL;
ZEND_FALLTHROUGH;
default:
Expand Down Expand Up @@ -2257,14 +2275,16 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext)
ldap_mods[i]->mod_bvalues[0]->bv_val = Z_STRVAL_P(value);
ldap_mods[i]->mod_bvalues[0]->bv_len = Z_STRLEN_P(value);
} else {
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;
}
if (!php_ldap_is_numerically_indexed_array(Z_ARRVAL_P(value))) {
zend_argument_value_error(3, "must be an array with numeric keys");
RETVAL_FALSE;
num_berval[i] = 0;
num_attribs = i + 1;
goto cleanup;
}

j = 0;
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(value), ivalue) {
convert_to_string(ivalue);
if (EG(exception)) {
num_berval[i] = j;
Expand All @@ -2275,7 +2295,8 @@ static void php_ldap_do_modify(INTERNAL_FUNCTION_PARAMETERS, int oper, int ext)
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);
}
j++;
} ZEND_HASH_FOREACH_END();
}
ldap_mods[i]->mod_bvalues[num_values] = NULL;
zend_hash_move_forward(Z_ARRVAL_P(entry));
Expand Down Expand Up @@ -2543,7 +2564,7 @@ PHP_FUNCTION(ldap_modify_batch)
zval *fetched;
char *dn;
size_t dn_len;
int i, j, k;
int i, j;
int num_mods, num_modprops, num_modvals;
LDAPMod **ldap_mods;
LDAPControl **lserverctrls = NULL;
Expand Down Expand Up @@ -2603,12 +2624,14 @@ PHP_FUNCTION(ldap_modify_batch)

num_mods = zend_hash_num_elements(Z_ARRVAL_P(mods));

for (i = 0; i < num_mods; i++) {
/* is the numbering consecutive? */
if ((fetched = zend_hash_index_find(Z_ARRVAL_P(mods), i)) == NULL) {
zend_argument_value_error(3, "must have consecutive integer indices starting from 0");
RETURN_THROWS();
}
if (!php_ldap_is_numerically_indexed_array(Z_ARRVAL_P(mods))) {
zend_argument_value_error(3, "must be an array with numeric keys");
RETURN_THROWS();
}

i = 0;
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(mods), fetched) {
ZVAL_DEREF(fetched);
mod = fetched;

/* is it an array? */
Expand Down Expand Up @@ -2706,19 +2729,10 @@ PHP_FUNCTION(ldap_modify_batch)
RETURN_THROWS();
}

/* are its keys integers? */
if (zend_hash_get_current_key_type(Z_ARRVAL_P(modinfo)) != HASH_KEY_IS_LONG) {
zend_value_error("%s(): Option \"" LDAP_MODIFY_BATCH_VALUES "\" must be integer-indexed", get_active_function_name());
if (!php_ldap_is_numerically_indexed_array(Z_ARRVAL_P(modinfo))) {
zend_value_error("%s(): Option \"" LDAP_MODIFY_BATCH_VALUES "\" must be an array with numeric keys", get_active_function_name());
RETURN_THROWS();
}

/* are the keys consecutive? */
for (k = 0; k < num_modvals; k++) {
if ((fetched = zend_hash_index_find(Z_ARRVAL_P(modinfo), k)) == NULL) {
zend_value_error("%s(): Option \"" LDAP_MODIFY_BATCH_VALUES "\" must have consecutive integer indices starting from 0", get_active_function_name());
RETURN_THROWS();
}
}
}

zend_hash_move_forward(Z_ARRVAL_P(mod));
Expand All @@ -2732,7 +2746,9 @@ PHP_FUNCTION(ldap_modify_batch)
zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_MODTYPE "\" is missing", get_active_function_name());
RETURN_THROWS();
}
}

i++;
} ZEND_HASH_FOREACH_END();
}
/* validation was successful */

Expand Down Expand Up @@ -2786,9 +2802,9 @@ PHP_FUNCTION(ldap_modify_batch)
ldap_mods[i]->mod_bvalues = safe_emalloc((num_modvals+1), sizeof(struct berval *), 0);

/* for each value */
for (j = 0; j < num_modvals; j++) {
j = 0;
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(vals), fetched) {
/* fetch it */
fetched = zend_hash_index_find(Z_ARRVAL_P(vals), j);
modval = zval_get_string(fetched);
if (EG(exception)) {
RETVAL_FALSE;
Expand All @@ -2804,7 +2820,8 @@ PHP_FUNCTION(ldap_modify_batch)
ldap_mods[i]->mod_bvalues[j]->bv_len = ZSTR_LEN(modval);
ldap_mods[i]->mod_bvalues[j]->bv_val = estrndup(ZSTR_VAL(modval), ZSTR_LEN(modval));
zend_string_release(modval);
}
j++;
} ZEND_HASH_FOREACH_END();

/* NULL-terminate values */
ldap_mods[i]->mod_bvalues[num_modvals] = NULL;
Expand Down
18 changes: 18 additions & 0 deletions ext/ldap/tests/gh17280.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--TEST--
GH-17280 (ldap_search() fails when $attributes array has holes)
--EXTENSIONS--
ldap
--FILE--
<?php

/* We are assuming 3333 is not connectable */
$ldap = ldap_connect('ldap://127.0.0.1:3333');

// Creating an array with a hole in it
$attr = array_unique(['cn', 'uid', 'uid', 'mail']);
var_dump(ldap_search($ldap, 'ou=people,dc=example,dc=com', 'uid=admin', $attr));

?>
--EXPECTF--
Warning: ldap_search(): Search: Can't contact LDAP server in %s on line %d
bool(false)
4 changes: 2 additions & 2 deletions ext/ldap/tests/ldap_add_error.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ try {
ldap_add($link, "dc=my-domain2,dc=com", array(
"objectClass" => array(
0 => "top",
2 => "dcObject",
"x" => "dcObject",
5 => "organization"),
"dc" => "my-domain",
"o" => "my-domain",
Expand Down Expand Up @@ -104,7 +104,7 @@ 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
ldap_add(): Argument #3 ($entry) must be an array with numeric keys

Warning: ldap_add(): Add: Undefined attribute type in %s on line %d
bool(false)
Expand Down
4 changes: 2 additions & 2 deletions ext/ldap/tests/ldap_search_error.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ $filter = "(dc=*)";
$result = ldap_search($link, $dn, $filter);
var_dump($result);

$result = ldap_search($link, $dn, $filter, array(1 => 'top'));
$result = ldap_search($link, $dn, $filter, array('foo' => 'top'));
var_dump($result);

try {
Expand Down Expand Up @@ -57,7 +57,7 @@ try {
Warning: ldap_search(): Search: No such object in %s on line %d
bool(false)

Warning: ldap_search(): Array initialization wrong in %s on line %d
Warning: ldap_search(): Argument #4 ($attributes) must be an array with numeric keys in %s on line %d
bool(false)
ldap_search(): Argument #1 ($ldap) cannot be empty
ldap_search(): Argument #2 ($base) must have the same number of elements as the links array
Expand Down

0 comments on commit c05268a

Please sign in to comment.