#49470 stack-buffer-overflow in slapi_pblock_get
Closed: wontfix 6 years ago Opened 6 years ago by firstyear.

Ticket was cloned from Red Hat Bugzilla (product Red Hat Enterprise Linux 7): Bug 1517980

Description of problem:
=================================================================
==26004== ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7fa0f0db93e0 at pc 0x7fa12a5fcfd2 bp 0x7fa0f0db9330 sp 0x7fa0f0db9320
WRITE of size 8 at 0x7fa0f0db93e0 thread T40
    #0 0x7fa12a5fcfd1 in slapi_pblock_get
/usr/src/debug/389-ds-base-1.3.7.5/ldap/servers/slapd/pblock.c:415
    #1 0x7fa12a5df9a7 in do_modify
/usr/src/debug/389-ds-base-1.3.7.5/ldap/servers/slapd/modify.c:285
    #2 0x56102e132e1c in ??
/usr/src/debug/389-ds-base-1.3.7.5/ldap/servers/slapd/connection.c:624
    #3 0x7fa1286f4c8a in PR_Select /usr/src/debug/nspr-4.17.0/pr/src/pthreads/.
./../../nspr/pr/src/pthreads/ptthread.c:216
    #4 0x7fa12abe9867 in _ZN6__asan10AsanThread11ThreadStartEv _asan_rtl_
    #5 0x7fa128094dd4 in start_thread
/usr/src/debug/glibc-2.17-c758a686/nptl/pthread_create.c:308
    #6 0x7fa1277429bc in __clone /usr/src/debug////////glibc-2.17-c758a686/misc
/../sysdeps/unix/sysv/linux/x86_64/clone.S:113
Address 0x7fa0f0db93e0 is located at offset 32 in frame <do_modify> of T40's
stack:
  This frame has 13 object(s):
    [32, 36) 'connid'
    [96, 100) 'opid'
    [160, 168) 'operation'
    [224, 232) 'pb_conn'
    [288, 296) 'len'
    [352, 360) 'normalized_mods'
    [416, 424) 'mod'
    [480, 488) 'last'
    [544, 552) 'type'
    [608, 616) 'old_pw'
    [672, 680) 'rawdn'
    [736, 760) 'smods'
    [800, 1312) 'ebuf'
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
Thread T40 created by T0 here:
    #0 0x7fa12abdaa0a in __interceptor_pthread_create _asan_rtl_
    #1 0x7fa1286f495b in PR_Select /usr/src/debug/nspr-4.17.0/pr/src/pthreads/.
./../../nspr/pr/src/pthreads/ptthread.c:457
    #2 0x0
Shadow bytes around the buggy address:
  0x0ff49e1af220: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff49e1af230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff49e1af240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff49e1af250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff49e1af260: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0ff49e1af270: 00 00 00 00 00 00 00 00 f1 f1 f1 f1[04]f4 f4 f4
  0x0ff49e1af280: f2 f2 f2 f2 04 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4
  0x0ff49e1af290: f2 f2 f2 f2 00 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4
  0x0ff49e1af2a0: f2 f2 f2 f2 00 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4
  0x0ff49e1af2b0: f2 f2 f2 f2 00 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4
  0x0ff49e1af2c0: f2 f2 f2 f2 00 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4
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
==26004== ABORTING

Version-Release number of selected component (if applicable):
389-ds-base-1.3.7.5-10.el7.x86_64

Metadata Update from @firstyear:
- Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1517980

6 years ago

Metadata Update from @firstyear:
- Issue assigned to firstyear

6 years ago

Metadata Update from @firstyear:
- Custom field component adjusted to None
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to review
- Custom field type adjusted to None
- Custom field version adjusted to None

6 years ago

That patch looks good.

Would we also need to change connid definition in 'struct op' in slap.h ?
Also the opid is an 'int' why changing it 'int' -> uint32 in modify.c

To be clear about the types we are using: PRUint64 == uint64_t, so we may as well be consistent.

Same with op - we are using int, but we shuold be explicit,

Actually, it should be int32_t ;)

@firstyear, I do not see much difference between the two patches :)

Anyway my main concern was related to opid that is widely store/retrieve as an 'int' so I was wondering why using here a int32. I think changing all opid get/set into int32 does not worth the effort. Your patch is good. ACK

Metadata Update from @tbordaz:
- Custom field reviewstatus adjusted to ack (was: review)

6 years ago

int == int32_t on x68_64. The issue is "other platforms". Int is "undefined size" .... I'm on a multi-year quest to use bounded types only in our server :) Small bits at a time .... :)

commit ab6d319
To ssh://git@pagure.io/389-ds-base.git
ef204a3..ab6d319 389-ds-base-1.3.7 -> 389-ds-base-1.3.7

commit 20efeea
To ssh://git@pagure.io/389-ds-base.git
b66b712..20efeea master -> master

Metadata Update from @firstyear:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

6 years ago

Metadata Update from @vashirov:
- Issue set to the milestone: None (was: 0.0 NEEDS_TRIAGE)

4 years ago

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/2529

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: fixed)

3 years ago

Login to comment on this ticket.

Metadata