#48982 Enabling a plugin live causes buffer overflow
Closed: Fixed None Opened 2 years ago by firstyear.

=================================================================
==26633==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6040000e593f at pc 0x7f7a08ddfe55 bp 0x7f79e5feb3d0 sp 0x7f79e5feab78
WRITE of size 53 at 0x6040000e593f thread T31
llvm-symbolizer: for the -functions option: Cannot find option named 'true'!
    #0 0x7f7a08ddfe54 in realpath /usr/src/debug/gcc-6.1.1-20160817/obj-x86_64-redhat-linux/x86_64-redhat-linux/libsanitizer/asan/../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:2810
    #1 0x43e4be in check_plugin_path /home/william/development/389ds/ds/ldap/servers/slapd/fedse.c:1960
    #2 0x7f7a08912e7f in dse_call_callback /home/william/development/389ds/ds/ldap/servers/slapd/dse.c:2634
    #3 0x7f7a08911c1c in dse_add /home/william/development/389ds/ds/ldap/servers/slapd/dse.c:2332
    #4 0x7f7a088d0910 in op_shared_add /home/william/development/389ds/ds/ldap/servers/slapd/add.c:700
    #5 0x7f7a088ced65 in do_add /home/william/development/389ds/ds/ldap/servers/slapd/add.c:227
    #6 0x41f5eb in connection_dispatch_operation /home/william/development/389ds/ds/ldap/servers/slapd/connection.c:612
    #7 0x4250e6 in connection_threadmain /home/william/development/389ds/ds/ldap/servers/slapd/connection.c:1759
    #8 0x7f7a05dcc7de in _pt_root /usr/src/debug/nspr-4.12.0/pr/src/pthreads/../../../nspr/pr/src/pthreads/ptthread.c:216
    #9 0x7f7a05b8c6b9 in start_thread pthread_create.c:?
    #10 0x7f7a058c73ce in __GI___clone :?

0x6040000e593f is located 0 bytes to the right of 47-byte region [0x6040000e5910,0x6040000e593f)
allocated by thread T31 here:
    #0 0x7f7a08e48f00 in malloc _asan_rtl_
    #1 0x7f7a088eb79c in slapi_ch_malloc /home/william/development/389ds/ds/ldap/servers/slapd/ch_malloc.c:113
    #2 0x43e4a7 in check_plugin_path /home/william/development/389ds/ds/ldap/servers/slapd/fedse.c:1959
    #3 0x7f7a08912e7f in dse_call_callback /home/william/development/389ds/ds/ldap/servers/slapd/dse.c:2634
    #4 0x7f7a08911c1c in dse_add /home/william/development/389ds/ds/ldap/servers/slapd/dse.c:2332
    #5 0x7f7a088d0910 in op_shared_add /home/william/development/389ds/ds/ldap/servers/slapd/add.c:700
    #6 0x7f7a088ced65 in do_add /home/william/development/389ds/ds/ldap/servers/slapd/add.c:227
    #7 0x41f5eb in connection_dispatch_operation /home/william/development/389ds/ds/ldap/servers/slapd/connection.c:612
    #8 0x4250e6 in connection_threadmain /home/william/development/389ds/ds/ldap/servers/slapd/connection.c:1759
    #9 0x7f7a05dcc7de in _pt_root /usr/src/debug/nspr-4.12.0/pr/src/pthreads/../../../nspr/pr/src/pthreads/ptthread.c:216

Thread T31 created by T0 here:
    #0 0x7f7a08db3538 in pthread_create _asan_rtl_
    #1 0x7f7a05dcc4ba in _PR_CreateThread /usr/src/debug/nspr-4.12.0/pr/src/pthreads/../../../nspr/pr/src/pthreads/ptthread.c:457

SUMMARY: AddressSanitizer: heap-buffer-overflow (/lib64/libasan.so.3+0x5de54)
Shadow bytes around the buggy address:
  0x0c0880014ad0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0880014ae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0880014af0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0880014b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0880014b10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c0880014b20: fa fa 00 00 00 00 00[07]fa fa 00 00 00 00 02 fa
  0x0c0880014b30: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
  0x0c0880014b40: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 02 fa
  0x0c0880014b50: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 04 fa
  0x0c0880014b60: fa fa 00 00 00 00 04 fa fa fa 00 00 00 00 02 fa
  0x0c0880014b70: fa fa 00 00 00 00 02 fa fa fa fd fd fd fd fd fa
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 right 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
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==26633==ABORTING

Isn't it false positive? full_path stores a full path which could contain some redundancies (e.g., '//'). realpath normalizes them and compacts the path?
{{{
1954 if ( vals[j] == '/' ) { / absolute path /
1955 full_path = slapi_get_plugin_name(NULL, vals[j]);
1956 } else { /
relative path */
1957 full_path = slapi_get_plugin_name(PLUGINDIR, vals[j]);
1958 }
1959 resolved_path = slapi_ch_malloc(strlen(full_path) + 1);
1960 res = realpath( full_path, resolved_path );
}}}

The plugin it affected had the name:

{{{
nsslapd-pluginPath: libro_replica
nsslapd-pluginPath: libhellorust
}}}

So I'm not really sure why this happened. I'll investigate further ....

Okay, the issue is in how we use realpath. The plugin is found as:

{{{
(gdb) print full_path
$3 = 0x6060001d61e0 "/opt/dirsrv/lib/dirsrv/plugins/libhellorust.so"
}}}

However, this is a versioned .so, so it's actually a symlink. When it's expanded, we go to:

{{{
(gdb) print res
$1 = 0x6040000f7910 "/opt/dirsrv/lib/dirsrv/plugins/libhellorust-0.1.0.so"
}}}

However, with realpath we are providing it the buffer to allocate into:

{{{
resolved_path = slapi_ch_malloc(strlen(full_path) + 1);
res = realpath( full_path, resolved_path );
}}}

This buffer is too small, thus the overflow occurs.

Either we can allocate 6 extra chars ((full_path) + 7) to fit in the "-0.0.0", or we can pass NULL to realpath, and just use free() on the result, rather than slapi_ch_free().

Given the symbol version could be valid as "-0.10.0" or even "-100.100.100", I think any static allocation scheme will fail. We should probably just dynamically allocate this, and use free().

Looks good!

According to the man page:
{{{
The free() function frees the memory space pointed to by ptr,
which must have been returned by a previous call to malloc(), calloc(),
or realloc(). Otherwise, or if free(ptr) has already been called
before, undefined behavior occurs. If ptr is NULL, no operation is
performed.
}}}
the following NULL check is no longer needed?
{{{
1981 if (res != NULL) {
}}}
If you are uncomfortable to remove the check, can we use slapi_ch_free_string instead of pure free although res was not malloc'ed slapi_ch_*alloc?

Otherwise, you have my ack.

commit 31c0425
Compressing objects: 100% (13/13), done.
Writing objects: 100% (13/13), 1.90 KiB | 0 bytes/s, done.
Total 13 (delta 9), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
04a9c89..31c0425 master -> master

commit 617b64f
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 690 bytes | 0 bytes/s, done.
Total 6 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
31c0425..617b64f master -> master

commit d945aba
Writing objects: 100% (6/6), 1.01 KiB | 0 bytes/s, done.
Total 6 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
9d66d3f..d945aba master -> master

Metadata Update from @firstyear:
- Issue assigned to firstyear
- Issue set to the milestone: 1.3.6 backlog

2 years ago

Login to comment on this ticket.

Metadata