Bug #76469 [Nab]: Bad call to ldap_bind not setting error in ldap_errno

Edit report at https://bugs.php.net/bug.php?id=76469&edit=1 ID: 76469 Updated by: cmb@php.net Reported by: clement dot oudot at worteks dot com Summary: Bad call to ldap_bind not setting error in ldap_errno Status: Not a bug Type: Bug Package: LDAP related Operating System: GNU/Linux PHP Version: 7.1.18 Assigned To: cmb Block user comment: N Private report: N New Comment: The documentation states[1]: | If the parameters given to a function are not what it expects, | such as passing an array where a string is expected, the return | value of the function is undefined. In this case it will likely | return NULL but this is just a convention, and cannot be relied | upon. I'm pretty sure that PHP will never return TRUE, so you can check the return value of ldap_bind(). However, it's better to ensure that you pass a string (or something compatible in weak type mode) in the first place. [1] <http://php.net/manual/en/functions.internal.php> Previous Comments: ------------------------------------------------------------------------ [2018-06-13 15:30:56] clement dot oudot at worteks dot com I understand the point, We need to check ldap_bind return code instead of relying on ldap_errno method. Maybe this could be added in documentation? ------------------------------------------------------------------------ [2018-06-13 14:34:42] nikic@php.net Not familiar with ldap_*, but most likely your "Actual result" is missing warnings related to illegal parameters. These come from parameter validation, which works the same for all internal functions and is unrelated to any error-reporting functionality specific to LDAP. As cmb mentioned, you can turn these into exceptions by setting strict_types=1 and I expect that we will make these always throw in PHP 8, but we're not going to be changing this in PHP 7.x. ------------------------------------------------------------------------ [2018-06-13 14:28:54] clement dot oudot at worteks dot com Looking at http://php.net/manual/en/function.ldap-errno.php, we see that errno should be set after each call to an LDAP command. In our case, the last LDAP command fails without setting an errno. Looks like a bug, no? ------------------------------------------------------------------------ [2018-06-13 14:21:53] cmb@php.net Thank you for taking the time to write to us, but this is not a bug. Please double-check the documentation available at http://www.php.net/manual/ and the instructions on how to report a bug at http://bugs.php.net/how-to-report.php Passing values of unsupported types to built-in functions is a userland programming error. If you want to be extra sure that this doesn't happen, use declare(strict_types=1). ------------------------------------------------------------------------ [2018-06-13 13:47:47] clement dot oudot at worteks dot com Description: ------------ When using an array as password when calling ldap_bind, we have a warning but ldap_errno is not reset, so we keep the value of the previous LDAP operation. As a lot of PHP code rely on ldap_errno to check if bind is successful, we a major security issue here: sending an array as GET/POST parameter to login age can bypass authentication if the code relies on errno. Test script: --------------- <?php error_reporting(0); $badpassword = "test"; $goodpassword = "secret"; $bugpassword[] = "a"; $ldap = ldap_connect("ldap://localhost"); ldap_set_option($ldap, LDAP_OPT_PROTOCOL_VERSION, 3); ldap_set_option($ldap, LDAP_OPT_REFERRALS, 0); $bind = ldap_bind( $ldap, "cn=admin,dc=example,dc=com" , $badpassword ); $errno = ldap_errno($ldap); echo "Bind 1 returns $errno\n"; $bind = ldap_bind( $ldap, "cn=admin,dc=example,dc=com" , $goodpassword ); $errno = ldap_errno($ldap); echo "Bind 2 returns $errno\n"; $bind = ldap_bind( $ldap, "cn=admin,dc=example,dc=com" , $bugpassword ); $errno = ldap_errno($ldap); echo "Bind 3 returns $errno\n"; Expected result: ---------------- Bind 1 returns 49 Bind 2 returns 0 Bind 3 returns 49 # or any error code Actual result: -------------- Bind 1 returns 49 Bind 2 returns 0 Bind 3 returns 0 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=76469&edit=1