Bug 13695 - ldb_filter_attrs() can corrupt memory if the DB is corrupt (seen in a samba-tool dbcheck)
ldb_filter_attrs() can corrupt memory if the DB is corrupt (seen in a samba-t...
Status: ASSIGNED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB
4.9.3
All Linux
: P5 normal
: ---
Assigned To: Andrew Bartlett
Samba QA Contact
https://gitlab.com/samba-team/samba/m...
:
Depends on: 14104
Blocks:
  Show dependency treegraph
 
Reported: 2018-11-29 17:34 UTC by Frank Rochlitzer
Modified: 2019-08-28 08:26 UTC (History)
3 users (show)

See Also:


Attachments
valgrind test (312.03 KB, text/plain)
2018-11-29 18:14 UTC, Frank Rochlitzer
no flags Details
possible fix and test (27.86 KB, patch)
2019-08-26 01:12 UTC, Andrew Bartlett
no flags Details
patch for master (v2) (30.78 KB, patch)
2019-08-26 04:45 UTC, Andrew Bartlett
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Rochlitzer 2018-11-29 17:34:57 UTC
We are using samba 4.7 and up servers as DC in our company.

We have found  a double free or corruption bug within samba-tool 4.9.2 on ArchLinux system.
On console type:
samba-tool dbcheck --cross-ncs --fix

Error:
On object CN=Testuser,CN=Users,DC=DOMAIN,DC=local
ERROR: Attribute usermaildirsize not present in replication metadata
Fix missing replPropertyMetaData element 'usermaildirsize' [y/N/all/none] y
double free or corruption (out)
===============================================================
double free or corruption (out)
===============================================================
INTERNAL ERROR: Signal 6 in pid 14520 (4.9.2)
Please read the Trouble-Shooting section of the Samba HOWTO
===============================================================
smb_panic_default: PANIC (pid 14520): internal error
BACKTRACE: 49 stack frames:
#0 /usr/lib/libsamba-util.so.0(log_stack_trace+0x33) [0x7ffadf8b3683]
#1 /usr/lib/libsamba-util.so.0(smb_panic+0x54) [0x7ffadf8b37b4]
#2 /usr/lib/libsamba-util.so.0(+0x249d7) [0x7ffadf8b39d7]
#3 /usr/lib/libpthread.so.0(+0x123c0) [0x7ffae0c593c0]
#4 /usr/lib/libc.so.6(gsignal+0x10f) [0x7ffae0abad7f]
#5 /usr/lib/libc.so.6(abort+0x125) [0x7ffae0aa5672]
#6 /usr/lib/libc.so.6(+0x7a878) [0x7ffae0afd878]
#7 /usr/lib/libc.so.6(+0x8118a) [0x7ffae0b0418a]
#8 /usr/lib/libc.so.6(+0x82c50) [0x7ffae0b05c50]
#9 /usr/lib/libtalloc.so.2(+0xa6f4) [0x7ffadf95f6f4]
#10 /usr/lib/libtalloc.so.2(+0xa560) [0x7ffadf95f560]
#11 /usr/lib/libtalloc.so.2(_talloc_free+0xd8) [0x7ffadf95a408]
#12 /usr/lib/ldb/libldb-key-value.so(ltdb_filter_attrs+0x245) [0x7ffad7f98ed5]
#13 /usr/lib/ldb/libldb-key-value.so(ltdb_search+0x41c) [0x7ffad7f9964c]
#14 /usr/lib/ldb/libldb-key-value.so(+0x6a29) [0x7ffad7f97a29]
#15 /usr/lib/libtevent.so.0(tevent_common_invoke_timer_handler+0xf7) [0x7ffadf94b6b7]
#16 /usr/lib/libtevent.so.0(tevent_common_loop_timer_delay+0x80) [0x7ffadf94b850]
#17 /usr/lib/libtevent.so.0(+0xc8ec) [0x7ffadf94c8ec]
#18 /usr/lib/libtevent.so.0(+0xad09) [0x7ffadf94ad09]
#19 /usr/lib/libtevent.so.0(_tevent_loop_once+0x85) [0x7ffadf946115]
#20 /usr/lib/libldb.so.1(ldb_wait+0xbc) [0x7ffadfb8a0ec]
#21 /usr/lib/python2.7/site-packages/ldb.so(+0xb651) [0x7ffadfba8651]
#22 /usr/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x6328) [0x7ffae072c1a8]
#23 /usr/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x2ca) [0x7ffae07810da]
#24 /usr/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x5f3f) [0x7ffae072bdbf]
#25 /usr/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x2ca) [0x7ffae07810da]
#26 /usr/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x5aff) [0x7ffae072b97f]
#27 /usr/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x2ca) [0x7ffae07810da]
#28 /usr/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x5aff) [0x7ffae072b97f]
#29 /usr/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x2ca) [0x7ffae07810da]
#30 /usr/lib/libpython2.7.so.1.0(+0xd2fb8) [0x7ffae0712fb8]
#31 /usr/lib/libpython2.7.so.1.0(PyObject_Call+0x43) [0x7ffae06ca0e3]
#32 /usr/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x33d9) [0x7ffae0729259]
#33 /usr/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x2ca) [0x7ffae07810da]
#34 /usr/lib/libpython2.7.so.1.0(+0xd2e0f) [0x7ffae0712e0f]
#35 /usr/lib/libpython2.7.so.1.0(PyObject_Call+0x43) [0x7ffae06ca0e3]
#36 /usr/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x33d9) [0x7ffae0729259]
#37 /usr/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x2ca) [0x7ffae07810da]
#38 /usr/lib/libpython2.7.so.1.0(+0xd2e0f) [0x7ffae0712e0f]
#39 /usr/lib/libpython2.7.so.1.0(PyObject_Call+0x43) [0x7ffae06ca0e3]
#40 /usr/lib/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x33d9) [0x7ffae0729259]
#41 /usr/lib/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x2ca) [0x7ffae07810da]
#42 /usr/lib/libpython2.7.so.1.0(PyEval_EvalCode+0x1a) [0x7ffae07a030a]
#43 /usr/lib/libpython2.7.so.1.0(+0x16ba81) [0x7ffae07aba81]
#44 /usr/lib/libpython2.7.so.1.0(PyRun_FileExFlags+0x87) [0x7ffae07ad397]
#45 /usr/lib/libpython2.7.so.1.0(PyRun_SimpleFileExFlags+0x194) [0x7ffae07adc84]
#46 /usr/lib/libpython2.7.so.1.0(Py_Main+0x603) [0x7ffae0789083]
#47 /usr/lib/libc.so.6(__libc_start_main+0xf3) [0x7ffae0aa7223]
#48 /usr/bin/python2(_start+0x2a) [0x5607af79377a]
Comment 1 Frank Rochlitzer 2018-11-29 17:39:20 UTC
Bug also exists in v4.9.3
Comment 2 Andrew Bartlett 2018-11-29 17:47:45 UTC
Marking this sensitive until we can triage it.

Can you get us some more info by running it under valgrind?

It will give lots of noise (grr, python....) but may give clues.
Comment 3 Frank Rochlitzer 2018-11-29 18:14:28 UTC
Created attachment 14703 [details]
valgrind test

valgrind --tool=memcheck -v --num-callers=20 --trace-children=yes --log-file=/var/log/samba-tool-%p.log /usr/bin/samba-tool dbcheck --cross-ncs --fix
Comment 4 Andrew Bartlett 2019-08-23 05:00:47 UTC
I've looked into this carefully, and I think we narrowly avoid a security bug here.

If the DB is corrupt, and an attribute exists in duplicate in the DB, then I think we can hit this.  

That is because in ldb_filter_attrs() / ltdb_filter_attrs() (the code has moved but is the same) has a loop that incorrectly accesses msg2->elements[num_elements] when num_elemenets == elements_size (the allocated array size). 

But only if there is a duplicate attribute can this matter, otherwise elements_size must be big enough (just...)
Comment 5 Andrew Bartlett 2019-08-26 01:12:40 UTC
Created attachment 15425 [details]
possible fix and test

This is under CI at the moment.
Comment 6 Andrew Bartlett 2019-08-26 04:44:30 UTC
This looks like a regression in 198471f9edfb9da2ee5b54e60a46d208f58ca2e4 in Samba 4.6
Comment 7 Andrew Bartlett 2019-08-26 04:45:04 UTC
Created attachment 15427 [details]
patch for master (v2)
Comment 8 Andrew Bartlett 2019-08-26 04:49:01 UTC
G'Day Frank,

I'm very sorry for how long this has taken for me to get to look into your issue.

This is a serious bug and I've developed tests and a fix.  I'm sure you have found another way to deal with your DB for now, but if you can test it this would be useful confirmation.