Skip to content

Backport fix GH-17280: ldap_search() fails when $attributes array has holes #17287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading