Skip to content

Commit 4c98b2c

Browse files
GirgiasGirgias
committed
ext/ldap: Improve validation of inputs for parallel search
1 parent 7cb2655 commit 4c98b2c

File tree

3 files changed

+103
-23
lines changed

3 files changed

+103
-23
lines changed

‎ext/ldap/ldap.c

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,54 +1485,66 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
14851485

14861486
/* parallel search? */
14871487
if (Z_TYPE_P(link) == IS_ARRAY) {
1488-
int i, nlinks, nbases, nfilters, *rcs;
1488+
int i, *rcs;
14891489
ldap_linkdata **lds;
14901490
zval *entry, object;
14911491

1492-
nlinks = zend_hash_num_elements(Z_ARRVAL_P(link));
1493-
if (nlinks == 0) {
1492+
uint32_t num_links = zend_hash_num_elements(Z_ARRVAL_P(link));
1493+
if (num_links == 0) {
14941494
zend_argument_must_not_be_empty_error(1);
14951495
ret = 0;
14961496
goto cleanup;
14971497
}
14981498

1499+
uint32_t num_base_dns = 0; /* If 0 this means we are working with a unique base dn */
14991500
if (base_dn_ht) {
1500-
nbases = zend_hash_num_elements(base_dn_ht);
1501-
if (nbases != nlinks) {
1502-
zend_argument_value_error(2, "must have the same number of elements as the links array");
1501+
if (!zend_array_is_list(base_dn_ht)) {
1502+
zend_argument_value_error(2, "must be a list");
1503+
RETURN_THROWS();
1504+
}
1505+
num_base_dns = zend_hash_num_elements(base_dn_ht);
1506+
if (num_base_dns != num_links) {
1507+
zend_argument_value_error(2, "must be the same size as argument #1");
15031508
ret = 0;
15041509
goto cleanup;
15051510
}
15061511
zend_hash_internal_pointer_reset(base_dn_ht);
15071512
} else {
1508-
nbases = 0; /* this means string, not array */
1509-
ldap_base_dn = zend_string_copy(base_dn_str);
1510-
if (EG(exception)) {
1513+
if (zend_str_has_nul_byte(base_dn_str)) {
1514+
zend_argument_value_error(2, "must not contain null bytes");
15111515
ret = 0;
15121516
goto cleanup;
15131517
}
1514-
// TODO check filter does not have any nul bytes
1518+
ldap_base_dn = zend_string_copy(base_dn_str);
15151519
}
15161520

1521+
uint32_t num_filters = 0; /* If 0 this means we are working with a unique base dn */
15171522
if (filter_ht) {
1518-
nfilters = zend_hash_num_elements(filter_ht);
1519-
if (nfilters != nlinks) {
1520-
zend_argument_value_error(3, "must have the same number of elements as the links array");
1523+
if (!zend_array_is_list(filter_ht)) {
1524+
zend_argument_value_error(3, "must be a list");
1525+
RETURN_THROWS();
1526+
}
1527+
num_filters = zend_hash_num_elements(filter_ht);
1528+
if (num_filters != num_links) {
1529+
zend_argument_value_error(3, "must be the same size as argument #1");
15211530
ret = 0;
15221531
goto cleanup;
15231532
}
15241533
zend_hash_internal_pointer_reset(filter_ht);
15251534
} else {
1526-
nfilters = 0; /* this means string, not array */
1535+
if (zend_str_has_nul_byte(filter_str)) {
1536+
zend_argument_value_error(3, "must not contain null bytes");
1537+
ret = 0;
1538+
goto cleanup;
1539+
}
15271540
ldap_filter = zend_string_copy(filter_str);
1528-
// TODO check filter does not have any nul bytes
15291541
}
15301542

1531-
lds = safe_emalloc(nlinks, sizeof(ldap_linkdata), 0);
1532-
rcs = safe_emalloc(nlinks, sizeof(*rcs), 0);
1543+
lds = safe_emalloc(num_links, sizeof(ldap_linkdata), 0);
1544+
rcs = safe_emalloc(num_links, sizeof(*rcs), 0);
15331545

15341546
zend_hash_internal_pointer_reset(Z_ARRVAL_P(link));
1535-
for (i=0; i<nlinks; i++) {
1547+
for (i=0; i<num_links; i++) {
15361548
entry = zend_hash_get_current_data(Z_ARRVAL_P(link));
15371549

15381550
if (Z_TYPE_P(entry) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(entry), ldap_link_ce)) {
@@ -1548,7 +1560,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15481560
goto cleanup_parallel;
15491561
}
15501562

1551-
if (nbases != 0) { /* base_dn an array? */
1563+
if (num_base_dns != 0) { /* base_dn an array? */
15521564
entry = zend_hash_get_current_data(base_dn_ht);
15531565
zend_hash_move_forward(base_dn_ht);
15541566
ldap_base_dn = zval_get_string(entry);
@@ -1558,7 +1570,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15581570
}
15591571
// TODO check dn does not have any nul bytes
15601572
}
1561-
if (nfilters != 0) { /* filter an array? */
1573+
if (num_filters != 0) { /* filter an array? */
15621574
entry = zend_hash_get_current_data(filter_ht);
15631575
zend_hash_move_forward(filter_ht);
15641576
ldap_filter = zval_get_string(entry);
@@ -1590,7 +1602,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15901602
array_init(return_value);
15911603

15921604
/* Collect results from the searches */
1593-
for (i=0; i<nlinks; i++) {
1605+
for (i=0; i<num_links; i++) {
15941606
if (rcs[i] != -1) {
15951607
rcs[i] = ldap_result(lds[i]->link, LDAP_RES_ANY, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res);
15961608
}

‎ext/ldap/tests/ldap_list_read_search_parallel_programming_errors.phpt

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
--TEST--
2+
Programming errors (Value/Type errors) for parallel usage of 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+
$ldaps = [$ldap, $ldap];
15+
16+
$dn = "dn_with\0nul_byte";
17+
try {
18+
var_dump(ldap_list($ldaps, $dn, $valid_filter));
19+
} catch (Throwable $e) {
20+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
21+
}
22+
23+
$filter = "filter_with\0nul_byte";
24+
try {
25+
var_dump(ldap_list($ldaps, $valid_dn, $filter));
26+
} catch (Throwable $e) {
27+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
28+
}
29+
30+
$not_list = [
31+
"attrib1",
32+
"wat" => "attrib2",
33+
];
34+
try {
35+
var_dump(ldap_list($ldaps, $not_list, $valid_filter));
36+
} catch (Throwable $e) {
37+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
38+
}
39+
try {
40+
var_dump(ldap_list($ldaps, $valid_dn, $not_list));
41+
} catch (Throwable $e) {
42+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
43+
}
44+
45+
$list_not_same_length = [
46+
"string1",
47+
"string2",
48+
"string3",
49+
];
50+
try {
51+
var_dump(ldap_list($ldaps, $list_not_same_length, $valid_filter));
52+
} catch (Throwable $e) {
53+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
54+
}
55+
try {
56+
var_dump(ldap_list($ldaps, $valid_dn, $list_not_same_length));
57+
} catch (Throwable $e) {
58+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
59+
}
60+
61+
?>
62+
--EXPECT--
63+
ValueError: ldap_list(): Argument #2 ($base) must not contain null bytes
64+
ValueError: ldap_list(): Argument #3 ($filter) must not contain null bytes
65+
ValueError: ldap_list(): Argument #2 ($base) must be a list
66+
ValueError: ldap_list(): Argument #3 ($filter) must be a list
67+
ValueError: ldap_list(): Argument #2 ($base) must be the same size as argument #1
68+
ValueError: ldap_list(): Argument #3 ($filter) must be the same size as argument #1

‎ext/ldap/tests/ldap_search_error.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ Warning: ldap_search(): Search: No such object in %s on line %d
6262
bool(false)
6363
ldap_search(): Argument #4 ($attributes) must be a list
6464
ldap_search(): Argument #1 ($ldap) must not be empty
65-
ldap_search(): Argument #2 ($base) must have the same number of elements as the links array
66-
ldap_search(): Argument #3 ($filter) must have the same number of elements as the links array
65+
ldap_search(): Argument #2 ($base) must be the same size as argument #1
66+
ldap_search(): Argument #3 ($filter) must be the same size as argument #1
6767
ldap_search(): Argument #2 ($base) must be of type string when argument #1 ($ldap) is an LDAP instance
6868
ldap_search(): Argument #3 ($filter) must be of type string when argument #1 ($ldap) is an LDAP instance

0 commit comments

Comments
 (0)