Skip to content

Commit 29a77e5

Browse files
GirgiasGirgias
committed
ext/ldap: Refactor validation of attributes array for php_ldap_do_search()
1 parent bca73f1 commit 29a77e5

File tree

3 files changed

+109
-26
lines changed

3 files changed

+109
-26
lines changed

‎ext/ldap/ldap.c‎

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,7 +1402,7 @@ static void php_set_opts(LDAP *ldap, int sizelimit, int timelimit, int deref, in
14021402
/* {{{ php_ldap_do_search */
14031403
static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
14041404
{
1405-
zval *link, *attrs = NULL, *attr, *serverctrls = NULL;
1405+
zval *link, *attrs = NULL, *serverctrls = NULL;
14061406
zend_string *base_dn_str, *filter_str;
14071407
HashTable *base_dn_ht, *filter_ht;
14081408
zend_long attrsonly, sizelimit, timelimit, deref;
@@ -1414,7 +1414,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
14141414
LDAPControl **lserverctrls = NULL;
14151415
int ldap_attrsonly = 0, ldap_sizelimit = -1, ldap_timelimit = -1, ldap_deref = -1;
14161416
int old_ldap_sizelimit = -1, old_ldap_timelimit = -1, old_ldap_deref = -1;
1417-
int num_attribs = 0, ret = 1, i, ldap_errno, argcount = ZEND_NUM_ARGS();
1417+
int ret = 1, ldap_errno, argcount = ZEND_NUM_ARGS();
14181418

14191419
ZEND_PARSE_PARAMETERS_START(3, 9)
14201420
Z_PARAM_ZVAL(link)
@@ -1444,29 +1444,45 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
14441444
case 5:
14451445
ldap_attrsonly = attrsonly;
14461446
ZEND_FALLTHROUGH;
1447-
case 4:
1448-
num_attribs = zend_hash_num_elements(Z_ARRVAL_P(attrs));
1449-
ldap_attrs = safe_emalloc((num_attribs+1), sizeof(char *), 0);
1447+
default:
1448+
break;
1449+
}
14501450

1451-
for (i = 0; i<num_attribs; i++) {
1452-
if ((attr = zend_hash_index_find(Z_ARRVAL_P(attrs), i)) == NULL) {
1453-
php_error_docref(NULL, E_WARNING, "Array initialization wrong");
1454-
ret = 0;
1455-
goto cleanup;
1456-
}
1451+
if (attrs) {
1452+
const HashTable *attributes = Z_ARRVAL_P(attrs);
1453+
uint32_t num_attribs = zend_hash_num_elements(attributes);
14571454

1458-
convert_to_string(attr);
1459-
if (EG(exception)) {
1460-
ret = 0;
1461-
goto cleanup;
1462-
}
1463-
ldap_attrs[i] = Z_STRVAL_P(attr);
1455+
if (num_attribs == 0) {
1456+
/* We don't allocate ldap_attrs for an empty array */
1457+
goto process;
1458+
}
1459+
if (!zend_array_is_list(attributes)) {
1460+
zend_argument_value_error(4, "must be a list");
1461+
RETURN_THROWS();
1462+
}
1463+
/* Allocate +1 as we need an extra entry to NULL terminate the list */
1464+
ldap_attrs = safe_emalloc(num_attribs+1, sizeof(char *), 0);
1465+
1466+
zend_ulong attribute_index = 0;
1467+
zval *attribute_zv = NULL;
1468+
ZEND_HASH_FOREACH_NUM_KEY_VAL(attributes, attribute_index, attribute_zv) {
1469+
ZVAL_DEREF(attribute_zv);
1470+
if (Z_TYPE_P(attribute_zv) != IS_STRING) {
1471+
zend_argument_type_error(4, "must be a list of strings, %s given", zend_zval_value_name(attribute_zv));
1472+
ret = 0;
1473+
goto cleanup;
14641474
}
1465-
ldap_attrs[num_attribs] = NULL;
1466-
ZEND_FALLTHROUGH;
1467-
default:
1468-
break;
1475+
zend_string *attribute = Z_STR_P(attribute_zv);
1476+
if (zend_str_has_nul_byte(attribute)) {
1477+
zend_argument_value_error(4, "must not contain strings with any null bytes");
1478+
ret = 0;
1479+
goto cleanup;
1480+
}
1481+
ldap_attrs[attribute_index] = ZSTR_VAL(attribute);
1482+
} ZEND_HASH_FOREACH_END();
1483+
ldap_attrs[num_attribs] = NULL;
14691484
}
1485+
process:
14701486

14711487
/* parallel search? */
14721488
if (Z_TYPE_P(link) == IS_ARRAY) {

‎ext/ldap/tests/ldap_list_read_search_programming_errors.phpt‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
--TEST--
2+
Programming errors (Value/Type errors) for ldap_list(), ldap_read(), and ldap_search()
3+
--EXTENSIONS--
4+
ldap
5+
--FILE--
6+
<?php
7+
8+
/* ldap_list(), ldap_read(), and ldap_search() share an underlying C function */
9+
/* We are assuming 3333 is not connectable */
10+
$ldap = ldap_connect('ldap://127.0.0.1:3333');
11+
$valid_dn = "cn=userA,something";
12+
$valid_filter = "";
13+
14+
$not_list = [
15+
"attrib1",
16+
"wat" => "attrib2",
17+
"attrib3",
18+
];
19+
try {
20+
var_dump(ldap_list($ldap, $valid_dn, $valid_filter, $not_list));
21+
} catch (Throwable $e) {
22+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
23+
}
24+
25+
$not_list_of_strings = [
26+
"attrib1",
27+
42,
28+
"attrib3",
29+
];
30+
try {
31+
var_dump(ldap_list($ldap, $valid_dn, $valid_filter, $not_list_of_strings));
32+
} catch (Throwable $e) {
33+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
34+
}
35+
36+
$list_of_strings_with_null_byte = [
37+
"attrib1",
38+
"attrib_with\0nul_byte",
39+
"attrib3",
40+
];
41+
try {
42+
var_dump(ldap_list($ldap, $valid_dn, $valid_filter, $list_of_strings_with_null_byte));
43+
} catch (Throwable $e) {
44+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
45+
}
46+
47+
$str = "attrib_with\0nul_byte";
48+
49+
$list_with_ref_nul_byte = [
50+
"attrib1",
51+
&$str,
52+
"attrib3",
53+
];
54+
try {
55+
var_dump(ldap_list($ldap, $valid_dn, $valid_filter, $list_with_ref_nul_byte));
56+
} catch (Throwable $e) {
57+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
58+
}
59+
60+
?>
61+
--EXPECT--
62+
ValueError: ldap_list(): Argument #4 ($attributes) must be a list
63+
TypeError: ldap_list(): Argument #4 ($attributes) must be a list of strings, int given
64+
ValueError: ldap_list(): Argument #4 ($attributes) must not contain strings with any null bytes
65+
ValueError: ldap_list(): Argument #4 ($attributes) must not contain strings with any null bytes

‎ext/ldap/tests/ldap_search_error.phpt‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,12 @@ $filter = "(dc=*)";
1919
$result = ldap_search($link, $dn, $filter);
2020
var_dump($result);
2121

22-
$result = ldap_search($link, $dn, $filter, array(1 => 'top'));
23-
var_dump($result);
22+
try {
23+
$result = ldap_search($link, $dn, $filter, array(1 => 'top'));
24+
var_dump($result);
25+
} catch (ValueError $exception) {
26+
echo $exception->getMessage() . "\n";
27+
}
2428

2529
try {
2630
ldap_search(array(), $dn, $filter, array('top'));
@@ -56,9 +60,7 @@ try {
5660
--EXPECTF--
5761
Warning: ldap_search(): Search: No such object in %s on line %d
5862
bool(false)
59-
60-
Warning: ldap_search(): Array initialization wrong in %s on line %d
61-
bool(false)
63+
ldap_search(): Argument #4 ($attributes) must be a list
6264
ldap_search(): Argument #1 ($ldap) must not be empty
6365
ldap_search(): Argument #2 ($base) must have the same number of elements as the links array
6466
ldap_search(): Argument #3 ($filter) must have the same number of elements as the links array

0 commit comments

Comments
 (0)