#48351 Fix buffer overflow error when reading url with len 0
Closed: Fixed None Opened 4 years ago by firstyear.

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

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);

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

2 years ago

Login to comment on this ticket.

Metadata