In ldaputil.c we perform the following:
char *urlcopy = slapi_ch_smprintf("%s%s%s", url_to_use, (url_to_use[len-1] == '/' ? "" : "/"), "");
However, it's possible to have url_to_use with a len of 0. This means we are reading from an undefined area of memory.
Found by gcc address sanitizer:
================================================================= ==30761== ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f5ecba8c2bf at pc 0x7f5ed5604517 bp 0x7fff1bb938f0 sp 0x7fff1bb938e0 READ of size 1 at 0x7f5ecba8c2bf thread T0 #0 0x7f5ed5604516 in slapi_ldap_url_parse /home/wibrown/development/389ds/ds/ldap/servers/slapd/ldaputil.c:342 #1 0x7f5ecba73dcc in cb_instance_hosturl_set /home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_instance.c:692 #2 0x7f5ecba78950 in cb_instance_config_set /home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_instance.c:1506 #3 0x7f5ecba7058a in cb_instance_config_set_default /home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_instance.c:145 #4 0x7f5ecba7ae4c in cb_create_default_backend_instance_config /home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_instance.c:1911 #5 0x7f5ecba651ec in cb_config_load_dse_info /home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_config.c:143 #6 0x7f5ecba8429c in chainingdb_start /home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_start.c:36 #7 0x7f5ed568c59d in plugin_call_func /home/wibrown/development/389ds/ds/ldap/servers/slapd/plugin.c:1920 #8 0x7f5ed568c2b8 in plugin_call_one /home/wibrown/development/389ds/ds/ldap/servers/slapd/plugin.c:1870 #9 0x7f5ed568b6bf in plugin_dependency_startall /home/wibrown/development/389ds/ds/ldap/servers/slapd/plugin.c:1679 #10 0x7f5ed568c149 in plugin_startall /home/wibrown/development/389ds/ds/ldap/servers/slapd/plugin.c:1832 #11 0x43de5b in main /home/wibrown/development/389ds/ds/ldap/servers/slapd/main.c:1054 #12 0x7f5ed2728af4 in __libc_start_main /usr/src/debug/glibc-2.17-c758a686/csu/libc-start.c:274 #13 0x40ee98 in _start (/opt/dirsrv/sbin/ns-slapd+0x40ee98) 0x7f5ecba8c2bf is located 1 bytes to the left of global variable '*.LC1 (/home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_instance.c)' (0x7f5ecba8c2c0) of size 1 '*.LC1 (/home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_instance.c)' is ascii string '' 0x7f5ecba8c2bf is located 47 bytes to the right of global variable '*.LC0 (/home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_instance.c)' (0x7f5ecba8c280) of size 16 '*.LC0 (/home/wibrown/development/389ds/ds/ldap/servers/plugins/chainingdb/cb_instance.c)' is ascii string 'nsFarmServerURL' SUMMARY: AddressSanitizer: global-buffer-overflow /home/wibrown/development/389ds/ds/ldap/servers/slapd/ldaputil.c:342 slapi_ldap_url_parse Shadow bytes around the buggy address: 0x0fec59749800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fec59749810: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fec59749820: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fec59749830: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fec59749840: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0fec59749850: 00 00 f9 f9 f9 f9 f9[f9]01 f9 f9 f9 f9 f9 f9 f9 0x0fec59749860: 00 00 04 f9 f9 f9 f9 f9 00 00 00 01 f9 f9 f9 f9 0x0fec59749870: 00 00 07 f9 f9 f9 f9 f9 02 f9 f9 f9 f9 f9 f9 f9 0x0fec59749880: 00 00 00 04 f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 0x0fec59749890: 00 00 00 00 f9 f9 f9 f9 02 f9 f9 f9 f9 f9 f9 f9 0x0fec597498a0: 00 00 06 f9 f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Heap righ redzone: fb Freed Heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack partial redzone: f4 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 ASan internal: fe ==30761== ABORTING
attachment 0001-Ticket-48351-Fix-buffer-overflow-error-when-reading-.patch
Looks good, but you forgot to put the ticket in "review?" state :-)
If you change the part, could you add the null check for url_to_use? If it's NULL, the server crashes there before coming your fixed code... (yes, your fix saves url_to_use = "\0" case, too.) 337 size_t len = strlen(url_to_use);
Add stronger NULL check 0001-Ticket-48351-Fix-buffer-overflow-error-when-reading-.2.patch
Thanks, William! Ack.
To ssh://git.fedorahosted.org/git/389/ds.git 284b167..8c17df6 master -> master commit 8c17df6
Metadata Update from @nhosoi: - Issue assigned to firstyear - Issue set to the milestone: 1.3.5.0
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/1682
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.