Skip to content

Commit fa100e1

Browse files
GirgiasGirgias
committed
ext/ldap: Improve validation of inputs for parallel search
1 parent 198b7e6 commit fa100e1

File tree

3 files changed

+105
-23
lines changed

3 files changed

+105
-23
lines changed

‎ext/ldap/ldap.c

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,12 +1485,12 @@ 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;
@@ -1501,43 +1501,57 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15011501
goto cleanup;
15021502
}
15031503

1504+
uint32_t num_base_dns = 0; /* If 0 this means we are working with a unique base dn */
15041505
if (base_dn_ht) {
1505-
nbases = zend_hash_num_elements(base_dn_ht);
1506-
if (nbases != nlinks) {
1507-
zend_argument_value_error(2, "must have the same number of elements as the links array");
1506+
if (!zend_array_is_list(base_dn_ht)) {
1507+
zend_argument_value_error(2, "must be a list");
1508+
ret = 0;
1509+
goto cleanup;
1510+
}
1511+
num_base_dns = zend_hash_num_elements(base_dn_ht);
1512+
if (num_base_dns != num_links) {
1513+
zend_argument_value_error(2, "must be the same size as argument #1");
15081514
ret = 0;
15091515
goto cleanup;
15101516
}
15111517
zend_hash_internal_pointer_reset(base_dn_ht);
15121518
} else {
1513-
nbases = 0; /* this means string, not array */
1514-
ldap_base_dn = zend_string_copy(base_dn_str);
1515-
if (EG(exception)) {
1519+
if (zend_str_has_nul_byte(base_dn_str)) {
1520+
zend_argument_value_error(2, "must not contain null bytes");
15161521
ret = 0;
15171522
goto cleanup;
15181523
}
1519-
// TODO check filter does not have any nul bytes
1524+
ldap_base_dn = zend_string_copy(base_dn_str);
15201525
}
15211526

1527+
uint32_t num_filters = 0; /* If 0 this means we are working with a unique base dn */
15221528
if (filter_ht) {
1523-
nfilters = zend_hash_num_elements(filter_ht);
1524-
if (nfilters != nlinks) {
1525-
zend_argument_value_error(3, "must have the same number of elements as the links array");
1529+
if (!zend_array_is_list(filter_ht)) {
1530+
zend_argument_value_error(3, "must be a list");
1531+
ret = 0;
1532+
goto cleanup;
1533+
}
1534+
num_filters = zend_hash_num_elements(filter_ht);
1535+
if (num_filters != num_links) {
1536+
zend_argument_value_error(3, "must be the same size as argument #1");
15261537
ret = 0;
15271538
goto cleanup;
15281539
}
15291540
zend_hash_internal_pointer_reset(filter_ht);
15301541
} else {
1531-
nfilters = 0; /* this means string, not array */
1542+
if (zend_str_has_nul_byte(filter_str)) {
1543+
zend_argument_value_error(3, "must not contain null bytes");
1544+
ret = 0;
1545+
goto cleanup;
1546+
}
15321547
ldap_filter = zend_string_copy(filter_str);
1533-
// TODO check filter does not have any nul bytes
15341548
}
15351549

1536-
lds = safe_emalloc(nlinks, sizeof(ldap_linkdata), 0);
1537-
rcs = safe_emalloc(nlinks, sizeof(*rcs), 0);
1550+
lds = safe_emalloc(num_links, sizeof(ldap_linkdata), 0);
1551+
rcs = safe_emalloc(num_links, sizeof(*rcs), 0);
15381552

15391553
zend_hash_internal_pointer_reset(Z_ARRVAL_P(link));
1540-
for (i=0; i<nlinks; i++) {
1554+
for (i=0; i<num_links; i++) {
15411555
entry = zend_hash_get_current_data(Z_ARRVAL_P(link));
15421556

15431557
if (Z_TYPE_P(entry) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(entry), ldap_link_ce)) {
@@ -1553,7 +1567,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15531567
goto cleanup_parallel;
15541568
}
15551569

1556-
if (nbases != 0) { /* base_dn an array? */
1570+
if (num_base_dns != 0) { /* base_dn an array? */
15571571
entry = zend_hash_get_current_data(base_dn_ht);
15581572
zend_hash_move_forward(base_dn_ht);
15591573
ldap_base_dn = zval_get_string(entry);
@@ -1563,7 +1577,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15631577
}
15641578
// TODO check dn does not have any nul bytes
15651579
}
1566-
if (nfilters != 0) { /* filter an array? */
1580+
if (num_filters != 0) { /* filter an array? */
15671581
entry = zend_hash_get_current_data(filter_ht);
15681582
zend_hash_move_forward(filter_ht);
15691583
ldap_filter = zval_get_string(entry);
@@ -1595,7 +1609,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15951609
array_init(return_value);
15961610

15971611
/* Collect results from the searches */
1598-
for (i=0; i<nlinks; i++) {
1612+
for (i=0; i<num_links; i++) {
15991613
if (rcs[i] != -1) {
16001614
rcs[i] = ldap_result(lds[i]->link, LDAP_RES_ANY, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res);
16011615
}

‎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)