Skip to content

Commit 56bd756

Browse files
lacatoirendossche
authored andcommitted
ext/ldap: fix crash in ldap_exop_sync() when $response_data is omitted
ldap_exop_sync() always takes the synchronous branch of php_ldap_exop(), which assigns $response_data unconditionally. When the argument is not passed, retdata is NULL and ZEND_TRY_ASSIGN_REF_STRINGL()/ ZEND_TRY_ASSIGN_REF_EMPTY_STRING() dereference it, so any successful ldap_exop_sync($ldap, $oid) call segfaults. Guard the assignment the way the $response_oid one already is. Closes GH-23339.
1 parent 7dd0ceb commit 56bd756

3 files changed

Lines changed: 29 additions & 2 deletions

File tree

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ PHP NEWS
111111
properties after running out of memory while escaping a property name.
112112
(adapik)
113113

114+
- LDAP:
115+
. Fix crash in ldap_exop_sync() when $response_data is omitted. (lacatoire)
116+
114117
- MBString:
115118
. Fixed bug GH-23958 (truncated encoding names resolving to full names). (Kamil Tekiela)
116119
. Fixed out-of-bounds read in php_mb_rfc1867_getword() on a multibyte lead

‎ext/ldap/ldap.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4129,10 +4129,12 @@ static void php_ldap_exop(INTERNAL_FUNCTION_PARAMETERS, bool force_sync) {
41294129
}
41304130

41314131
if (lretdata) {
4132-
ZEND_TRY_ASSIGN_REF_STRINGL(retdata, lretdata->bv_val, lretdata->bv_len);
4132+
if (retdata) {
4133+
ZEND_TRY_ASSIGN_REF_STRINGL(retdata, lretdata->bv_val, lretdata->bv_len);
4134+
}
41334135
ldap_memfree(lretdata->bv_val);
41344136
ldap_memfree(lretdata);
4135-
} else {
4137+
} else if (retdata) {
41364138
ZEND_TRY_ASSIGN_REF_EMPTY_STRING(retdata);
41374139
}
41384140

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
ldap_exop_sync() - without the response_data argument
3+
--EXTENSIONS--
4+
ldap
5+
--SKIPIF--
6+
<?php require_once('skipifbindfailure.inc'); ?>
7+
--FILE--
8+
<?php
9+
require "connect.inc";
10+
11+
$link = ldap_connect_and_bind($uri, $user, $passwd, $protocol_version);
12+
13+
var_dump(
14+
ldap_exop_sync($link, LDAP_EXOP_WHO_AM_I),
15+
ldap_exop_sync($link, LDAP_EXOP_WHO_AM_I, null, null, $retdata),
16+
$retdata
17+
);
18+
?>
19+
--EXPECTF--
20+
bool(true)
21+
bool(true)
22+
string(%d) "dn:%s"

0 commit comments

Comments
 (0)