Bug 14138 (CVE-2019-14861) - CVE-2019-14861 [SECURITY] DNSServer RPC server crash
Summary: CVE-2019-14861 [SECURITY] DNSServer RPC server crash
Status: RESOLVED FIXED
Alias: CVE-2019-14861
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.10.8
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL: https://lists.samba.org/archive/samba...
Keywords:
Depends on:
Blocks: 14185
  Show dependency tree / graph
 
Reported: 2019-09-23 04:07 UTC by Andrew Bartlett
Modified: 2024-04-11 00:20 UTC (History)
5 users (show)

See Also:

Attachments
patch without tests for master (5.34 KB, patch)
2019-10-21 03:11 UTC, Andrew Bartlett 		abartlet: ci-passed+
Details
patch for master (v2) (11.68 KB, patch)
2019-10-29 04:45 UTC, Andrew Bartlett 		no flags Details
patch for master (v3) (5.34 KB, patch)
2019-10-30 02:15 UTC, Andrew Bartlett 		no flags Details
patch for master (v4) (16.99 KB, patch)
2019-10-30 02:17 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: ci-passed+
Details
patch for Samba 4.9 (cherry-picked from master patch) v4 (17.07 KB, patch)
2019-10-30 02:19 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: review+
Details
patch for Samba 4.10 (cherry-picked from master patch) v4 (17.07 KB, patch)
2019-10-30 02:19 UTC, Andrew Bartlett 		dbagnall: ci-passed-
Details
patch for Samba 4.11 (cherry-picked from master patch) v4 (16.99 KB, patch)
2019-10-30 02:20 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: review+
abartlet: ci-passed+
Details
first draft security advisory (2.24 KB, text/plain)
2019-10-30 03:16 UTC, Andrew Bartlett 		no flags Details
patch for Samba 4.10 (cherry-picked from master patch) v5 (17.23 KB, patch)
2019-10-31 01:38 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: review+
abartlet: ci-passed+
Details
draft security advisory v2 (2.15 KB, text/plain)
2019-10-31 02:12 UTC, Douglas Bagnall 		abartlet: review+
Details
patch for master (v6) (17.00 KB, patch)
2019-11-03 21:37 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: ci-passed+
Details
patch for Samba 4.11 (cherry-picked from master patch) v6 (18.67 KB, patch)
2019-11-03 21:37 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: ci-passed+
Details
patch for Samba 4.10 (cherry-picked from master patch) v6 (18.91 KB, patch)
2019-11-03 21:38 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: ci-passed+
Details
patch for Samba 4.9 (cherry-picked from master patch) v6 (18.75 KB, patch)
2019-11-03 21:39 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: ci-passed+
Details
draft security advisory v3 (2.15 KB, text/plain)
2019-11-29 10:24 UTC, Karolin Seeger 		kseeger: review+
Details
draft security advisory v4 (2.16 KB, text/plain)
2019-11-29 11:01 UTC, Karolin Seeger 		abartlet: review+
kseeger: review+
Details
draft security advisory v5 (2.18 KB, text/plain)
2019-12-09 12:03 UTC, Arvid Requate 		no flags Details
Show Obsolete (11) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2019-09-23 04:07:31 UTC 
(from Andreas Oster in the samba mailing list post above)

Hi all,

I am currently having some problems managing DNS on our Samba servers
with bind9 backend. Whenever I try to access the AD domain zone from the
MS DNS tool the DNS part crashes and services need to be restarted.
After the crash DNS is not working anymore.

I get the following in log.samba:

[2019/09/20 09:39:10.681745,  0]
../../source4/rpc_server/dnsserver/dcerpc_dnsserver.c:1101(dnsserver_query_zone)
  dnsserver: Invalid zone operation
IsSigned===============================================================
[2019/09/20 09:39:10.693900,  0] ../../lib/util/fault.c:80(fault_report)
  INTERNAL ERROR: Signal 11 in pid 6563 (4.10.9-GIT-e3f51924971)
  Please read the Trouble-Shooting section of the Samba HOWTO
[2019/09/20 09:39:10.694089,  0] ../../lib/util/fault.c:82(fault_report)
  ===============================================================
[2019/09/20 09:39:10.694168,  0]
../../lib/util/fault.c:128(smb_panic_default)
  smb_panic_default: PANIC (pid 6563): internal error
[2019/09/20 09:39:10.695626,  0] ../../lib/util/fault.c:261(log_stack_trace)
  BACKTRACE: 49 stack frames:
   #0 /usr/local/samba/lib/libsamba-util.so.0(log_stack_trace+0x2e)
[0x7fdc478da67f]
   #1 /usr/local/samba/lib/libsamba-util.so.0(+0x22405) [0x7fdc478da405]
   #2 /usr/local/samba/lib/libsamba-util.so.0(log_stack_trace+0)
[0x7fdc478da651]
   #3 /usr/local/samba/lib/libsamba-util.so.0(+0x22307) [0x7fdc478da307]
   #4 /usr/local/samba/lib/libsamba-util.so.0(+0x2231c) [0x7fdc478da31c]
   #5 /lib/x86_64-linux-gnu/libpthread.so.0(+0x12890) [0x7fdc45fcf890]
   #6
/usr/local/samba/lib/private/libldb.so.1(ldb_msg_find_element+0x5e)
[0x7fdc483c57d3]
   #7
/usr/local/samba/lib/private/libldb.so.1(ldb_msg_find_ldb_val+0x23)
[0x7fdc483c68ba]
   #8
/usr/local/samba/lib/private/libldb.so.1(ldb_msg_find_attr_as_string+0x27)
[0x7fdc483c6f80]
   #9 /usr/local/samba/lib/libdcerpc-server.so.0(dns_name_compare+0x4e)
[0x7fdc30b97f22]
   #10 /usr/local/samba/lib/private/libldb.so.1(ldb_qsort+0x31e)
[0x7fdc483d6953]
   #11 /usr/local/samba/lib/libdcerpc-server.so.0(+0x848dc) [0x7fdc30b918dc]
   #12 /usr/local/samba/lib/libdcerpc-server.so.0(+0x85df4) [0x7fdc30b92df4]
   #13 /usr/local/samba/lib/libdcerpc-server.so.0(+0x867b7) [0x7fdc30b937b7]
   #14 /usr/local/samba/lib/libdcerpc-server.so.0(+0x2ac58) [0x7fdc30b37c58]
   #15 /usr/local/samba/lib/libdcerpc-server.so.0(+0x2bcda) [0x7fdc30b38cda]
   #16 /usr/local/samba/lib/libdcerpc-server.so.0(+0x2d6fc) [0x7fdc30b3a6fc]
   #17
Comment 1 Andreas Oster 2019-09-26 07:10:00 UTC 
If this turns into a security advisory, I'm happy to be credited as 'Andreas Oster'
Comment 2 Andrew Bartlett 2019-10-16 02:41:49 UTC 
This looks like a bug in ldb_qsort().

I was able to reproduce it thanks to the great assistance of Andreas and when I patch ldb_qsort() for qsort_r() the problem goes away.

Sadly qsort_r() is not standard but there are existing patterns such as 
https://github.com/noporpoise/sort_r to cope with the two dominant implementations.
Comment 3 Andrew Bartlett 2019-10-16 04:21:29 UTC 
I've proposed this to avoid using ldb_qsort() elsewhere:  

https://gitlab.com/samba-team/samba/merge_requests/848

However I still need to find a good workaround here.  I'm assuming that our ldb_qsort() code isn't totally broken, but that the dns_name_compare() function is just too complex or not consistent enough as a comparison function.
Comment 4 Andrew Bartlett 2019-10-16 05:32:06 UTC 
I think I've found the case of the issue in terms of the data, which should allow us to reproduce this independently.

Andreas had in a zone, let's call it "samba.org"

There was an @ record, and the "samba.org" record in that zone. 

Testing against windows shows that the "samba.org" record is ignored, if created directly with LDAP, and modifications over DNSserver instead point back to the @ record.

I'm assuming that the issue is due to the unstable sort in dns_name_compare(), as both @ and 'samba.org' want to be sorted first. 

So a fix is likely to:

 - stop sorting the "samba.org" record first in dns_name_compare()
 - ignore "samba.org" records in dns_build_tree().  A fallback could be to do that unless there has been no @ record (for compatibility)
 - We can likely also stop sorting @ first, as dns_build_tree() already has a special case for that.

This will then allow us to also not use ldb_qsort() as it will not be needed any longer, we can just qsort() instead. 

DNS records over LDAP are able to be added with few permissions, so we will need a CVE.
Comment 5 Andrew Bartlett 2019-10-16 18:01:36 UTC 
I've asked Red Hat for a CVE.
Comment 6 Andrew Bartlett 2019-10-21 03:11:53 UTC 
Created attachment 15560 [details]
patch without tests for master

This patch addresses the basic issue by removing the ldb_qsort(). 

I need to add tests that are run against windows, then finish removing the special cases for @.
Comment 7 Andrew Bartlett 2019-10-21 03:58:31 UTC 
Comment on attachment 15560 [details]
patch without tests for master

This patch is wrong, windows does sort the result in the enum case.

I'll however do that by making the tree insertion be a binary insert rather than trying to do it with the qsort.
Comment 8 Andrew Bartlett 2019-10-29 04:45:30 UTC 
Created attachment 15574 [details]
patch for master (v2)

Patch with tests for master
Comment 9 Andrew Bartlett 2019-10-29 04:52:20 UTC 
A CVSS score:

CVSS:3.1/AV:N/AC:H/PR:L/UI:N/S:U/C:N/I:N/A:H (5.3)
Comment 10 Andrew Bartlett 2019-10-30 02:15:54 UTC 
Created attachment 15579 [details]
patch for master (v3)
Comment 11 Andrew Bartlett 2019-10-30 02:17:31 UTC 
Created attachment 15580 [details]
patch for master (v4)
Comment 12 Andrew Bartlett 2019-10-30 02:19:21 UTC 
Created attachment 15581 [details]
patch for Samba 4.9 (cherry-picked from master patch) v4
Comment 13 Andrew Bartlett 2019-10-30 02:19:44 UTC 
Created attachment 15582 [details]
patch for Samba 4.10 (cherry-picked from master patch) v4
Comment 14 Andrew Bartlett 2019-10-30 02:20:21 UTC 
Created attachment 15583 [details]
patch for Samba 4.11 (cherry-picked from master patch) v4
Comment 15 Andrew Bartlett 2019-10-30 03:16:44 UTC 
Created attachment 15584 [details]
first draft security advisory
Comment 16 Andrew Bartlett 2019-10-31 01:38:59 UTC 
Created attachment 15590 [details]
patch for Samba 4.10 (cherry-picked from master patch) v5

The 4.10 patch needs flapping entries for the python2 duplicate tests.
Comment 17 Douglas Bagnall 2019-10-31 01:48:21 UTC 
Comment on attachment 15580 [details]
patch for master (v4)

I'll add another patch for master to fix a comment typo, so as to avoid another round of CI and review.
Comment 18 Douglas Bagnall 2019-10-31 02:12:52 UTC 
Created attachment 15591 [details]
draft security advisory v2
Comment 19 Andrew Bartlett 2019-10-31 07:08:08 UTC 
G'Day Karolin,

A heads up that this is almost ready, just waiting for the final CI on 4.9 (traditionally problematic, but I'm sure I'll get a pass eventually).
Comment 20 Andrew Bartlett 2019-11-03 21:37:25 UTC 
Created attachment 15595 [details]
patch for master (v6)
Comment 21 Andrew Bartlett 2019-11-03 21:37:56 UTC 
Created attachment 15596 [details]
patch for Samba 4.11 (cherry-picked from master patch) v6
Comment 22 Andrew Bartlett 2019-11-03 21:38:33 UTC 
Created attachment 15597 [details]
patch for Samba 4.10 (cherry-picked from master patch) v6
Comment 23 Andrew Bartlett 2019-11-03 21:39:23 UTC 
Created attachment 15598 [details]
patch for Samba 4.9 (cherry-picked from master patch) v6

Updated patches include the flapping workaround and the spelling fix in all branches.
Comment 24 Andrew Bartlett 2019-11-05 01:13:58 UTC 
Assigning to Karolin for the next available security release.
Comment 25 Karolin Seeger 2019-11-20 11:10:06 UTC 
(In reply to Andrew Bartlett from comment #24)
Planned release date: Tuesday, December 10 2019
Comment 26 Karolin Seeger 2019-11-29 10:11:56 UTC 
Opening bug report for vendors.
Planned release date: December 10
Comment 27 Karolin Seeger 2019-11-29 10:24:10 UTC 
Created attachment 15647 [details]
draft security advisory v3

"acting at an AD DC" -> "acting as an AD DC", "4.9.16" -> "4.9.17"
Comment 28 Karolin Seeger 2019-11-29 11:01:25 UTC 
Created attachment 15649 [details]
draft security advisory v4

Fix CVE number
Comment 29 Andrew Bartlett 2019-11-29 17:53:00 UTC 
Comment on attachment 15649 [details]
draft security advisory v4

Thanks for fixing those up.
Comment 30 Arvid Requate 2019-12-09 12:03:48 UTC 
Created attachment 15674 [details]
draft security advisory v5

Fixes minor wording issue in advisory summary.
Comment 31 Karolin Seeger 2019-12-10 09:09:26 UTC 
(In reply to Arvid Requate from comment #30)
Hi Arvid,

thanks for the changes, it's very much appreciated!
Sorry for not picking it up, because it was too late in the release process (included in the release notes).

Karolin
Comment 32 Karolin Seeger 2019-12-10 09:10:32 UTC 
Samba 4.11.3, 4.10.11 and 4.9.17 have been shipped to address this defect.
Comment 33 Karolin Seeger 2019-12-10 09:18:36 UTC 
Pushed to autobuild-master.
Comment 34 Karolin Seeger 2019-12-10 09:27:39 UTC 
Pushed to v4-{11,10,9}-test.
Comment 35 Karolin Seeger 2019-12-13 08:54:30 UTC 
Pushed to master.
Closing out bug report.

Thanks!
Comment 36 Douglas Bagnall 2024-04-11 00:20:45 UTC 
Opening to non-vendors, and removing samba-vendors@samba.org from CCs. This would normally have happened when the patches went out.

Vendors: you can visit the bug and add yourselves individually to the CC list if you so wish.