https://bugs.openldap.org/show_bug.cgi?id=10130
Issue ID: 10130 Summary: Several callers of getpassphrase() ignore NULL returns Product: OpenLDAP Version: 2.6.6 Hardware: All OS: All Status: UNCONFIRMED Keywords: needs_review Severity: normal Priority: --- Component: slapd Assignee: bugs@openldap.org Reporter: stacey.marshall@gmail.com Target Milestone: ---
getpassphrase(3c) and lutil_getpass() can return NULL to signify EOF, and in the case of the former for an interrupt or an error. Several callers fail to check for NULL before calling other functions which may then cause other issues such as segmentation fault.
A patch in progress treats NULL as EOF and provides an early exit.
``` $ git status --short -uno M clients/tools/common.c M clients/tools/ldappasswd.c M clients/tools/ldapvc.c M servers/slapd/slappasswd.c M tests/progs/slapd-tester.c ```
https://bugs.openldap.org/show_bug.cgi?id=10130
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords|needs_review | Target Milestone|--- |2.5.17
https://bugs.openldap.org/show_bug.cgi?id=10130
--- Comment #1 from stacey.marshall@gmail.com --- Created attachment 988 --> https://bugs.openldap.org/attachment.cgi?id=988&action=edit Fixes 10130 Several callers of getpassphrase() ignore NULL returns
The attached file is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in the following patch(es) were developed by Oracle. Oracle has not assigned rights and/or interest in this work to any party. I, Stacey Marshall am authorized by Oracle, my employer, to release this work under the following terms.
Oracle hereby place the following modifications to OpenLDAP Software (and only these modifications) into the public domain. Hence, these modifications may be freely used and/or redistributed for any purpose with or without attribution and/or other notice.
https://bugs.openldap.org/show_bug.cgi?id=10130
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|bugs@openldap.org |hyc@openldap.org
https://bugs.openldap.org/show_bug.cgi?id=10130
--- Comment #2 from Howard Chu hyc@openldap.org --- Why bother with these? If the user wants to exit, they can use Ctrl-C just as easily as Ctrl-D (EOF). There's no benefit to these changes.
https://bugs.openldap.org/show_bug.cgi?id=10130
--- Comment #3 from stacey.marshall@gmail.com --- In testing, where ^D was being entered for password requests, we were seeing core files created.
We were applying a number of fixes to other software packages and thought that we'd also address OpenLDAP.
https://bugs.openldap.org/show_bug.cgi?id=10130
--- Comment #4 from Howard Chu hyc@openldap.org --- Still that's an abnormal use case, when an interrupt is the usual way to exit. For abnormal situations, I see nothing wrong with generating a core file.
In this chunk
diff --git a/clients/tools/ldapvc.c b/clients/tools/ldapvc.c index 4f35025ec..badbdb947 100644 --- a/clients/tools/ldapvc.c +++ b/clients/tools/ldapvc.c @@ -309,8 +309,14 @@ main( int argc, char *argv[] ) #endif && !cred.bv_val) { - cred.bv_val = strdup(getpassphrase(_("User's password: "))); - cred.bv_len = strlen(cred.bv_val); + char *userpw = getpassphrase(_("User's password: ")); + if ( userpw == NULL ) /* Allow EOF to exit. */ + { + free( cred.bv_val ); + tool_exit( ld, EXIT_FAILURE ); + } + cred.bv_val = strdup(userpw); + cred.bv_len = strlen(cred.bv_val); }
#ifdef LDAP_API_FEATURE_VERIFY_CREDENTIALS_INTERACTIVE
the free( cred.bv_val ) is unnecessary, since this code is inside an if ( !cred.bv_val ).
https://bugs.openldap.org/show_bug.cgi?id=10130
--- Comment #5 from stacey.marshall@gmail.com --- (In reply to Howard Chu from comment #4)
Still that's an abnormal use case, when an interrupt is the usual way to exit. For abnormal situations, I see nothing wrong with generating a core file.
For the user using the commands, perhaps not, an administrator may wonder why these core files are being generated.
``` # pstack /var/cores/core.slapd.521808.1700478973 core '/var/cores/core.slapd.521808.1700478973' of 1626: slappasswd 00007feec4a6069a __lwp_sigqueue () + a 00007feec4992d0c raise () + 1c 00007feec495acb1 abort () + e1 00007feec495bf11 _assert_c99 () + 81 00007feec5165992 ch_strdup (0x7ff044c69760) + a2 00007feec51eb26a slappasswd (0x7ff044c69940, 0x7ff044c69940) + 68a 00007feec5114a06 main (0x7ff044c69950, 0x7ff044c69950) + 166 00007feec51146f4 ???????? ()
# pstack /var/cores/core.openldappasswd.521808.1700477840 core '/var/cores/core.openldappasswd.521808.1700477840' of 1602: ldappasswd -A 00007fdc94b43e70 strlen () + 30 00007fdc9510ac92 main (0x7fe78c0bd4a0, 0x7fe78c0bd4a0) + 162 00007fdc9510a524 ???????? () ```
In this chunk
the free( cred.bv_val ) is unnecessary, since this code is inside an if ( !cred.bv_val ).
Agreed.
https://bugs.openldap.org/show_bug.cgi?id=10130
Howard Chu hyc@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |IN_PROGRESS Ever confirmed|0 |1 Severity|normal |trivial Priority|--- |Lowest
--- Comment #6 from Howard Chu hyc@openldap.org --- https://git.openldap.org/openldap/openldap/-/merge_requests/669
https://bugs.openldap.org/show_bug.cgi?id=10130
--- Comment #7 from Quanah Gibson-Mount quanah@openldap.org --- head:
• 8139458b by Stacey Marshall at 2024-01-10T18:47:36+00:00 ITS#10130 Several callers of getpassphrase() ignore NULL returns
• cf21be22 by Howard Chu at 2024-01-10T18:47:36+00:00 ITS#10130 cleanup prev commit
https://bugs.openldap.org/show_bug.cgi?id=10130
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |TEST Status|IN_PROGRESS |RESOLVED
https://bugs.openldap.org/show_bug.cgi?id=10130
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|TEST |FIXED
--- Comment #8 from Quanah Gibson-Mount quanah@openldap.org --- RE26:
• 8cb36ebc by Stacey Marshall at 2024-01-16T20:36:57+00:00 ITS#10130 Several callers of getpassphrase() ignore NULL returns
• 43798262 by Stacey Marshall at 2024-01-16T20:37:56+00:00 ITS#10130 Several callers of getpassphrase() ignore NULL returns
https://bugs.openldap.org/show_bug.cgi?id=10130
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |VERIFIED
-
openldap-its＠openldap.org