https://bugs.openldap.org/show_bug.cgi?id=10155
Issue ID: 10155 Summary: Invalid [aka FUZZ] -F and -T options can core dump ldapsearch Product: OpenLDAP Version: 2.6.6 Hardware: All OS: All Status: UNCONFIRMED Keywords: needs_review Severity: normal Priority: --- Component: client tools Assignee: bugs@openldap.org Reporter: doug.leavitt@oracle.com Target Milestone: ---
A customer reported core dumps in ldapsearch which has been tracked to the the improper use of the -F and -T options.
The customer confirmed removing the invalid -F and -T options from their script eliminated the core dumps.
The CLI arguments of the failing ldapsearch look like:
ldapsearch <good CLI args> -F , -T u <good filter and attr args> The good CLI args include proper uses of -H... -x -D ... -w ... -b ... -s ... The good filter and attrs are also valid CLI inputs.
The "bad" args are <sp>-F<sp><COMMA><sp>-T<sp>-u<sp> The -u is also valid but it is consumed as a directory name of -T
From man page and code review the the -F argument is supposed to be a valid URL. and the -T argument is supposed to be a valid directory
The core file output indicates that main calls free after the search takes place. The location is believed to be here:
1658 if ( urlpre != NULL ) { 1659 if ( def_urlpre != urlpre ) 1660 free( def_urlpre ); <--------- 1661 free( urlpre ); 1662 } ... 1672 tool_exit( ld, rc ); ...
This is the first example of the use of -F we have seen so it is unclear how this should be fixed.
But code review of ldapsearch.c and common.c exposed a few weaknesses that could help in addressing the issue.
Observed weaknesses:
The getopt processing code for -T does not check that the arg is actually a directory and fail/error when bad input is provided. Perhaps at least an access(2) check should be performed?
It is unclear if -F should only accept file:// URLs. The existing code does not sufficiently check any URL format instead it processes the argument by looking for the first '/' [no error checking] and determine the remainder to be a tmpdir location similar to the -T argument. So, Fuzz input of <COMMA> seems to eventually lead to the core files.
It is unclear if -F and -T should be mutually exclusive or not.
It seems like the fix to this issue is to add better error checking and to fail on FUZZ inputs. I defer a solution to upstream as it probably requires project direction I lack.
https://bugs.openldap.org/show_bug.cgi?id=10155
doug.leavitt@oracle.com doug.leavitt@oracle.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Hardware|All |Other OS|All |Solaris
https://bugs.openldap.org/show_bug.cgi?id=10155
--- Comment #1 from doug.leavitt@oracle.com doug.leavitt@oracle.com --- The bug may also exist on x86, but we lack access to the actual LDAP server DB to replicate.
https://bugs.openldap.org/show_bug.cgi?id=10155
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |2.5.18 Keywords|needs_review |
https://bugs.openldap.org/show_bug.cgi?id=10155
Howard Chu hyc@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Status|UNCONFIRMED |CONFIRMED
--- Comment #2 from Howard Chu hyc@openldap.org --- The -T / -F options were added back in 1999 b73b0c61582166d37d55a90067c5783d2164af39
(Actually -V was added, but was renamed to -F some years later.)
There's not much explanation or rationale, and indeed they were only documented in the manpage several years later, 2006.
But from what I can tell: the options are not mutually exclusive. -T controls where temp values are actually written, and -F controls what is written to the LDIF to describe where the values were written. There's no particular reason why these paths must coincide, particularly if the LDIF files or tmp files are later copied to somewhere else.
Frankly we don't really care about fuzzing for one-shot commandline tools. Feeding bogus input doesn't break anything or affect anyone other than the user running the tool.
"-T -u" may be valid, certainly "-u" may be a valid directory name.
The only real issue here is that free() may be called on urlpre which may refer directly to an optarg, as opposed to being a strdup of optarg, and so causes a SEGV on exit. That will be fixed, but again nobody really cares about a SEGV on exit from a one-shot command.
https://bugs.openldap.org/show_bug.cgi?id=10155
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|2.5.18 |2.5.19
https://bugs.openldap.org/show_bug.cgi?id=10155
Ondřej Kuzník ondra@mistotebe.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|bugs@openldap.org |ondra@mistotebe.net Status|CONFIRMED |IN_PROGRESS
--- Comment #3 from Ondřej Kuzník ondra@mistotebe.net --- https://git.openldap.org/openldap/openldap/-/merge_requests/729
https://bugs.openldap.org/show_bug.cgi?id=10155
Quanah Gibson-Mount quanah@openldap.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|IN_PROGRESS |RESOLVED
--- Comment #4 from Quanah Gibson-Mount quanah@openldap.org --- head: • e2910559 by Ondřej Kuzník at 2024-10-28T17:39:53+00:00 ITS#10155 manage option values more carefully
https://bugs.openldap.org/show_bug.cgi?id=10155
--- Comment #5 from Quanah Gibson-Mount quanah@openldap.org --- RE26:
• 00ff5b35 by Ondřej Kuzník at 2024-11-12T17:49:21+00:00 ITS#10155 manage option values more carefully • 00ff5b35
RE25:
• 6909574c by Ondřej Kuzník at 2024-11-12T17:49:42+00:00 ITS#10155 manage option values more carefully