Bug 13695 - ldb_filter_attrs() can corrupt memory if the DB is corrupt (seen in a samba-tool dbcheck)
Summary: ldb_filter_attrs() can corrupt memory if the DB is corrupt (seen in a samba-t...
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.9.3
Hardware: All Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL: https://gitlab.com/samba-team/samba/m...
Depends on: 14104
  Show dependency treegraph
Reported: 2018-11-29 17:34 UTC by Frank Rochlitzer
Modified: 2020-02-03 01:46 UTC (History)
3 users (show)

See Also:

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

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.
Comment 9 Frank Rochlitzer 2020-01-27 18:36:36 UTC
Hey Andrew,

I've tested now with a self compiled 4.11.2 version where your patch was included.
My origin error is gone but now I've got:

ERROR: Attribute mailbox not present in replication metadata
Fix missing replPropertyMetaData element 'mailbox' [y/N/all/none] y
ERROR(ldb): uncaught exception - ldb_wait from (null) with LDB_WAIT_ALL: Operations error (1)
  File "/usr/lib/python3.8/site-packages/samba/netcmd/__init__.py", line 186, in _run
    return self.run(*args, **kwargs)
  File "/usr/lib/python3.8/site-packages/samba/netcmd/dbcheck.py", line 169, in run
    error_count = chk.check_database(DN=DN, scope=search_scope,
  File "/usr/lib/python3.8/site-packages/samba/dbchecker.py", line 259, in check_database
    error_count += self.check_object(object.dn, attrs=attrs)
  File "/usr/lib/python3.8/site-packages/samba/dbchecker.py", line 2564, in check_object
    self.fix_metadata(obj, att)
  File "/usr/lib/python3.8/site-packages/samba/dbchecker.py", line 1573, in fix_metadata
    res = self.samdb.search(base=dn, scope=ldb.SCOPE_BASE, attrs=[attr],
Comment 10 Andrew Bartlett 2020-02-03 01:46:01 UTC
Thanks, that is the expected result.  We now error (sorry the message isn't particularly clear or useful) rather than writing to invalid memory.

This is one of those unique cases where I will suggest editing the backend DB.  I think your only option is to delete and re-create that entry.

You won't be able to do that against sam.ldb, you will need to do it against the files in sam.ldb.d/ 

When done, 'samba-tool dbcheck --reindex' and then 'samba-tool dbcheck --fix' will ensure the indices are correct.

A search on that entry with ldbsearch, delete with ldbdel and re-add with ldbadd should work. 

A dump of the DB with ldbdump might be a good idea and help show what the DB really looks like (in terms of those duplicate attributes).

I still don't know how this got to be corrupt, and we may never know, but I thank you for your patience.