Skip to content

Commit 491fab0

Browse files
GirgiasGirgias
committed
ext/ldap: Improve validation of inputs for parallel search
1 parent 29a77e5 commit 491fab0

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
@@ -1486,12 +1486,12 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
14861486

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

1493-
nlinks = zend_hash_num_elements(Z_ARRVAL_P(link));
1494-
if (nlinks == 0) {
1493+
uint32_t num_links = zend_hash_num_elements(Z_ARRVAL_P(link));
1494+
if (num_links == 0) {
14951495
zend_argument_must_not_be_empty_error(1);
14961496
ret = 0;
14971497
goto cleanup;
@@ -1502,43 +1502,57 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15021502
goto cleanup;
15031503
}
15041504

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

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

1537-
lds = safe_emalloc(nlinks, sizeof(ldap_linkdata), 0);
1538-
rcs = safe_emalloc(nlinks, sizeof(*rcs), 0);
1551+
lds = safe_emalloc(num_links, sizeof(ldap_linkdata), 0);
1552+
rcs = safe_emalloc(num_links, sizeof(*rcs), 0);
15391553

15401554
zend_hash_internal_pointer_reset(Z_ARRVAL_P(link));
1541-
for (i=0; i<nlinks; i++) {
1555+
for (i=0; i<num_links; i++) {
15421556
entry = zend_hash_get_current_data(Z_ARRVAL_P(link));
15431557

15441558
if (Z_TYPE_P(entry) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(entry), ldap_link_ce)) {
@@ -1554,7 +1568,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15541568
goto cleanup_parallel;
15551569
}
15561570

1557-
if (nbases != 0) { /* base_dn an array? */
1571+
if (num_base_dns != 0) { /* base_dn an array? */
15581572
entry = zend_hash_get_current_data(base_dn_ht);
15591573
zend_hash_move_forward(base_dn_ht);
15601574
ldap_base_dn = zval_get_string(entry);
@@ -1564,7 +1578,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15641578
}
15651579
// TODO check dn does not have any nul bytes
15661580
}
1567-
if (nfilters != 0) { /* filter an array? */
1581+
if (num_filters != 0) { /* filter an array? */
15681582
entry = zend_hash_get_current_data(filter_ht);
15691583
zend_hash_move_forward(filter_ht);
15701584
ldap_filter = zval_get_string(entry);
@@ -1596,7 +1610,7 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15961610
array_init(return_value);
15971611

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

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