diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index 3fea267d14ff9..90c665f38f496 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -1402,11 +1402,12 @@ static void php_set_opts(LDAP *ldap, int sizelimit, int timelimit, int deref, in /* {{{ php_ldap_do_search */ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope) { - zval *link, *attrs = NULL, *attr, *serverctrls = NULL; - zend_string *base_dn_str, *filter_str; - HashTable *base_dn_ht, *filter_ht; + zval *link, *attrs = NULL, *serverctrls = NULL; + HashTable *base_dn_ht = NULL; + zend_string *base_dn_str = NULL; + HashTable *filter_ht = NULL; + zend_string *filter_str = NULL; zend_long attrsonly, sizelimit, timelimit, deref; - zend_string *ldap_filter = NULL, *ldap_base_dn = NULL; char **ldap_attrs = NULL; ldap_linkdata *ld = NULL; ldap_resultdata *result; @@ -1414,7 +1415,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope) LDAPControl **lserverctrls = NULL; int ldap_attrsonly = 0, ldap_sizelimit = -1, ldap_timelimit = -1, ldap_deref = -1; int old_ldap_sizelimit = -1, old_ldap_timelimit = -1, old_ldap_deref = -1; - int num_attribs = 0, ret = 1, i, ldap_errno, argcount = ZEND_NUM_ARGS(); + int ret = 1, ldap_errno, argcount = ZEND_NUM_ARGS(); ZEND_PARSE_PARAMETERS_START(3, 9) Z_PARAM_ZVAL(link) @@ -1444,142 +1445,190 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope) case 5: ldap_attrsonly = attrsonly; ZEND_FALLTHROUGH; - case 4: - num_attribs = zend_hash_num_elements(Z_ARRVAL_P(attrs)); - ldap_attrs = safe_emalloc((num_attribs+1), sizeof(char *), 0); + default: + break; + } - for (i = 0; ilink) { + ldap_linkdata *current_ld = Z_LDAP_LINK_P(link_zv); + if (!current_ld->link) { zend_throw_error(NULL, "LDAP connection has already been closed"); ret = 0; goto cleanup_parallel; } - if (nbases != 0) { /* base_dn an array? */ - entry = zend_hash_get_current_data(base_dn_ht); - zend_hash_move_forward(base_dn_ht); - ldap_base_dn = zval_get_string(entry); - if (EG(exception)) { + if (num_base_dns != 0) { /* base_dn an array? */ + zval *base_dn_zv = zend_hash_index_find(base_dn_ht, ldap_link_index); + ZEND_ASSERT(base_dn_zv); + ZVAL_DEREF(base_dn_zv); + if (Z_TYPE_P(base_dn_zv) != IS_STRING) { + zend_argument_type_error(2, "must be a list of strings, %s given", zend_zval_value_name(base_dn_zv)); + ret = 0; + goto cleanup_parallel; + } + ldap_base_dn = Z_STR_P(base_dn_zv); + if (zend_str_has_nul_byte(ldap_base_dn)) { + zend_argument_value_error(2, "must not contain null bytes"); ret = 0; goto cleanup_parallel; } - // TODO check dn does not have any nul bytes } - if (nfilters != 0) { /* filter an array? */ - entry = zend_hash_get_current_data(filter_ht); - zend_hash_move_forward(filter_ht); - ldap_filter = zval_get_string(entry); - if (EG(exception)) { + if (num_filters != 0) { /* filter an array? */ + zval *filter_zv = zend_hash_index_find(filter_ht, ldap_link_index); + ZEND_ASSERT(filter_zv); + ZVAL_DEREF(filter_zv); + if (Z_TYPE_P(filter_zv) != IS_STRING) { + zend_argument_type_error(3, "must be a list of strings, %s given", zend_zval_value_name(filter_zv)); + ret = 0; + goto cleanup_parallel; + } + ldap_filter = Z_STR_P(filter_zv); + if (zend_str_has_nul_byte(ldap_filter)) { + zend_argument_value_error(3, "must not contain null bytes"); ret = 0; goto cleanup_parallel; } - // TODO check filter does not have any nul bytes } if (serverctrls) { /* We have to parse controls again for each link as they use it */ _php_ldap_controls_free(&lserverctrls); - lserverctrls = _php_ldap_controls_from_array(ld->link, serverctrls, 9); + lserverctrls = _php_ldap_controls_from_array(current_ld->link, serverctrls, 9); if (lserverctrls == NULL) { - rcs[i] = -1; + rcs[ldap_link_index] = -1; + // TODO Throw an exception/cleanup? continue; } } - php_set_opts(ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref); + php_set_opts(current_ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref); /* Run the actual search */ - ldap_search_ext(ld->link, ZSTR_VAL(ldap_base_dn), scope, ZSTR_VAL(ldap_filter), ldap_attrs, ldap_attrsonly, lserverctrls, NULL, NULL, ldap_sizelimit, &rcs[i]); - lds[i] = ld; - zend_hash_move_forward(Z_ARRVAL_P(link)); - } + ldap_search_ext(current_ld->link, ZSTR_VAL(ldap_base_dn), scope, ZSTR_VAL(ldap_filter), ldap_attrs, ldap_attrsonly, lserverctrls, NULL, NULL, ldap_sizelimit, &rcs[ldap_link_index]); + lds[ldap_link_index] = current_ld; + + // TODO Reset the options of the link? + } ZEND_HASH_FOREACH_END(); array_init(return_value); /* Collect results from the searches */ - for (i=0; ilink, LDAP_RES_ANY, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res); } if (rcs[i] != -1) { + zval object; object_init_ex(&object, ldap_result_ce); result = Z_LDAP_RESULT_P(&object); result->result = ldap_res; @@ -1601,18 +1650,16 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope) } if (!base_dn_str) { - zend_argument_type_error(2, "must be of type string when argument #1 ($ldap) is an LDAP instance"); + zend_argument_type_error(2, "must be of type string when argument #1 ($ldap) is an LDAP\\Connection instance"); ret = 0; goto cleanup; } - ldap_base_dn = zend_string_copy(base_dn_str); if (!filter_str) { - zend_argument_type_error(3, "must be of type string when argument #1 ($ldap) is an LDAP instance"); + zend_argument_type_error(3, "must be of type string when argument #1 ($ldap) is an LDAP\\Connection instance"); ret = 0; goto cleanup; } - ldap_filter = zend_string_copy(filter_str); if (serverctrls) { lserverctrls = _php_ldap_controls_from_array(ld->link, serverctrls, 9); @@ -1625,7 +1672,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope) php_set_opts(ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref); /* Run the actual search */ - ldap_errno = ldap_search_ext_s(ld->link, ZSTR_VAL(ldap_base_dn), scope, ZSTR_VAL(ldap_filter), ldap_attrs, ldap_attrsonly, lserverctrls, NULL, NULL, ldap_sizelimit, &ldap_res); + ldap_errno = ldap_search_ext_s(ld->link, ZSTR_VAL(base_dn_str), scope, ZSTR_VAL(filter_str), ldap_attrs, ldap_attrsonly, lserverctrls, NULL, NULL, ldap_sizelimit, &ldap_res); if (ldap_errno != LDAP_SUCCESS && ldap_errno != LDAP_SIZELIMIT_EXCEEDED @@ -1657,7 +1704,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope) result->result = ldap_res; } } else { - zend_argument_type_error(1, "must be of type LDAP|array, %s given", zend_zval_value_name(link)); + zend_argument_type_error(1, "must be of type LDAP\\Connection|array, %s given", zend_zval_value_name(link)); } cleanup: @@ -1665,12 +1712,6 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope) /* Restoring previous options */ php_set_opts(ld->link, old_ldap_sizelimit, old_ldap_timelimit, old_ldap_deref, &ldap_sizelimit, &ldap_timelimit, &ldap_deref); } - if (ldap_filter) { - zend_string_release(ldap_filter); - } - if (ldap_base_dn) { - zend_string_release(ldap_base_dn); - } if (ldap_attrs != NULL) { efree(ldap_attrs); } diff --git a/ext/ldap/tests/gh16101.phpt b/ext/ldap/tests/gh16101.phpt new file mode 100644 index 0000000000000..1e9d57334378b --- /dev/null +++ b/ext/ldap/tests/gh16101.phpt @@ -0,0 +1,25 @@ +--TEST-- +Bug GH-16101: Segfault in ldap_list(), ldap_read(), and ldap_search() when LDAPs array is not a list +--EXTENSIONS-- +ldap +--FILE-- + $ldap, + "world" => $ldap, +]; +try { + var_dump(ldap_list($ldaps_dict, $valid_dn, $valid_filter)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +ValueError: ldap_list(): Argument #1 ($ldap) must be a list diff --git a/ext/ldap/tests/ldap_list_read_search_parallel_programming_errors.phpt b/ext/ldap/tests/ldap_list_read_search_parallel_programming_errors.phpt new file mode 100644 index 0000000000000..4f3567d9e00b7 --- /dev/null +++ b/ext/ldap/tests/ldap_list_read_search_parallel_programming_errors.phpt @@ -0,0 +1,113 @@ +--TEST-- +Programming errors (Value/Type errors) for parallel usage of ldap_list(), ldap_read(), and ldap_search() +--EXTENSIONS-- +ldap +--FILE-- +getMessage(), PHP_EOL; +} + +$dn = "dn_with\0nul_byte"; +try { + var_dump(ldap_list($ldaps, $dn, $valid_filter)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$filter = "filter_with\0nul_byte"; +try { + var_dump(ldap_list($ldaps, $valid_dn, $filter)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$not_list = [ + "attrib1", + "wat" => "attrib2", +]; +try { + var_dump(ldap_list($ldaps, $not_list, $valid_filter)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump(ldap_list($ldaps, $valid_dn, $not_list)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$list_not_same_length = [ + "string1", + "string2", + "string3", +]; +try { + var_dump(ldap_list($ldaps, $list_not_same_length, $valid_filter)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump(ldap_list($ldaps, $valid_dn, $list_not_same_length)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$list_not_all_strings = [ + 42, + "string2", +]; +try { + var_dump(ldap_list($ldaps, $list_not_all_strings, $valid_filter)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump(ldap_list($ldaps, $valid_dn, $list_not_all_strings)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$list_strings_with_nul_bytes = [ + "string\0nul_byte", + "string2", +]; +try { + var_dump(ldap_list($ldaps, $list_strings_with_nul_bytes, $valid_filter)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + var_dump(ldap_list($ldaps, $valid_dn, $list_strings_with_nul_bytes)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +ValueError: ldap_list(): Argument #1 ($ldap) must be a list of LDAP\Connection +ValueError: ldap_list(): Argument #2 ($base) must not contain null bytes +ValueError: ldap_list(): Argument #3 ($filter) must not contain null bytes +ValueError: ldap_list(): Argument #2 ($base) must be a list +ValueError: ldap_list(): Argument #3 ($filter) must be a list +ValueError: ldap_list(): Argument #2 ($base) must be the same size as argument #1 +ValueError: ldap_list(): Argument #3 ($filter) must be the same size as argument #1 +TypeError: ldap_list(): Argument #2 ($base) must be a list of strings, int given +TypeError: ldap_list(): Argument #3 ($filter) must be a list of strings, int given +ValueError: ldap_list(): Argument #2 ($base) must not contain null bytes +ValueError: ldap_list(): Argument #3 ($filter) must not contain null bytes \ No newline at end of file diff --git a/ext/ldap/tests/ldap_list_read_search_parallel_references.phpt b/ext/ldap/tests/ldap_list_read_search_parallel_references.phpt new file mode 100644 index 0000000000000..321cb1cff6c1c --- /dev/null +++ b/ext/ldap/tests/ldap_list_read_search_parallel_references.phpt @@ -0,0 +1,37 @@ +--TEST-- +Programming errors (Value/Type errors) for parallel usage of ldap_list(), ldap_read(), and ldap_search() with references +--EXTENSIONS-- +ldap +--FILE-- +getMessage(), PHP_EOL; +} +try { + var_dump(ldap_list($ldaps, $valid_dn, $list_with_ref_nul_byte)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +ValueError: ldap_list(): Argument #2 ($base) must not contain null bytes +ValueError: ldap_list(): Argument #3 ($filter) must not contain null bytes diff --git a/ext/ldap/tests/ldap_list_read_search_programming_errors.phpt b/ext/ldap/tests/ldap_list_read_search_programming_errors.phpt new file mode 100644 index 0000000000000..59e227d818ecf --- /dev/null +++ b/ext/ldap/tests/ldap_list_read_search_programming_errors.phpt @@ -0,0 +1,86 @@ +--TEST-- +Programming errors (Value/Type errors) for ldap_list(), ldap_read(), and ldap_search() +--EXTENSIONS-- +ldap +--FILE-- +getMessage(), PHP_EOL; +} + +try { + var_dump(ldap_list($ldap, [$valid_dn], $valid_filter)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +try { + var_dump(ldap_list($ldap, $valid_dn, [$valid_filter])); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$not_list = [ + "attrib1", + "wat" => "attrib2", + "attrib3", +]; +try { + var_dump(ldap_list($ldap, $valid_dn, $valid_filter, $not_list)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$not_list_of_strings = [ + "attrib1", + 42, + "attrib3", +]; +try { + var_dump(ldap_list($ldap, $valid_dn, $valid_filter, $not_list_of_strings)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$list_of_strings_with_null_byte = [ + "attrib1", + "attrib_with\0nul_byte", + "attrib3", +]; +try { + var_dump(ldap_list($ldap, $valid_dn, $valid_filter, $list_of_strings_with_null_byte)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +$str = "attrib_with\0nul_byte"; + +$list_with_ref_nul_byte = [ + "attrib1", + &$str, + "attrib3", +]; +try { + var_dump(ldap_list($ldap, $valid_dn, $valid_filter, $list_with_ref_nul_byte)); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +TypeError: ldap_list(): Argument #1 ($ldap) must be of type LDAP\Connection|array, int given +TypeError: ldap_list(): Argument #2 ($base) must be of type string when argument #1 ($ldap) is an LDAP\Connection instance +TypeError: ldap_list(): Argument #3 ($filter) must be of type string when argument #1 ($ldap) is an LDAP\Connection instance +ValueError: ldap_list(): Argument #4 ($attributes) must be a list +TypeError: ldap_list(): Argument #4 ($attributes) must be a list of strings, int given +ValueError: ldap_list(): Argument #4 ($attributes) must not contain strings with any null bytes +ValueError: ldap_list(): Argument #4 ($attributes) must not contain strings with any null bytes diff --git a/ext/ldap/tests/ldap_search_error.phpt b/ext/ldap/tests/ldap_search_error.phpt index 4e775ad13d66e..04d8af46f5982 100644 --- a/ext/ldap/tests/ldap_search_error.phpt +++ b/ext/ldap/tests/ldap_search_error.phpt @@ -19,8 +19,12 @@ $filter = "(dc=*)"; $result = ldap_search($link, $dn, $filter); var_dump($result); -$result = ldap_search($link, $dn, $filter, array(1 => 'top')); -var_dump($result); +try { + $result = ldap_search($link, $dn, $filter, array(1 => 'top')); + var_dump($result); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} try { ldap_search(array(), $dn, $filter, array('top')); @@ -56,11 +60,9 @@ try { --EXPECTF-- 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 -bool(false) +ldap_search(): Argument #4 ($attributes) must be a list ldap_search(): Argument #1 ($ldap) must not be empty -ldap_search(): Argument #2 ($base) must have the same number of elements as the links array -ldap_search(): Argument #3 ($filter) must have the same number of elements as the links array -ldap_search(): Argument #2 ($base) must be of type string when argument #1 ($ldap) is an LDAP instance -ldap_search(): Argument #3 ($filter) must be of type string when argument #1 ($ldap) is an LDAP instance +ldap_search(): Argument #2 ($base) must be the same size as argument #1 +ldap_search(): Argument #3 ($filter) must be the same size as argument #1 +ldap_search(): Argument #2 ($base) must be of type string when argument #1 ($ldap) is an LDAP\Connection instance +ldap_search(): Argument #3 ($filter) must be of type string when argument #1 ($ldap) is an LDAP\Connection instance