Bug 13922 (CVE-2019-12435) - CVE-2019-12435 [SECURITY] zone operations can crash rpc server
Summary: CVE-2019-12435 [SECURITY] zone operations can crash rpc server
Status: RESOLVED FIXED
Alias: CVE-2019-12435
Product: Samba 4.1 and newer
Classification: Unclassified
Component: DCE-RPCs and pipes (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks: 13990
  Show dependency tree / graph
 
Reported: 2019-05-02 05:35 UTC by Douglas Bagnall
Modified: 2019-06-19 17:27 UTC (History)
6 users (show)

See Also:

Attachments
first draft security advisory (1.79 KB, text/plain)
2019-05-19 22:29 UTC, Andrew Bartlett 		no flags Details
patch for master (5.60 KB, patch)
2019-05-22 01:34 UTC, Douglas Bagnall 		no flags Details
patch for master v2 (6.43 KB, patch)
2019-05-29 06:44 UTC, Aaron Haslett (dead mail address) 		abartlet: review+
Details
Security Advisory (1.76 KB, text/plain)
2019-05-29 06:46 UTC, Aaron Haslett (dead mail address) 		no flags Details
Patch for master (6.60 KB, patch)
2019-05-31 05:09 UTC, Joe Guo (mail address dead) 		no flags Details
Backport patch for v4-10-test (6.60 KB, patch)
2019-05-31 05:12 UTC, Joe Guo (mail address dead) 		joeg: ci-passed+
Details
Backport patch for v4-9-test (6.60 KB, patch)
2019-05-31 05:13 UTC, Joe Guo (mail address dead) 		joeg: ci-passed+
Details
Updated advisory (v02) (1.77 KB, text/plain)
2019-06-08 09:03 UTC, Douglas Bagnall 		abartlet: review+
Details
patch for master (v3) (6.57 KB, patch)
2019-06-08 14:41 UTC, Andrew Bartlett 		dbagnall: review+
Details
patch for Samba 4.10 (v3) (6.57 KB, patch)
2019-06-08 14:45 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: review+
abartlet: ci-passed+
Details
patch for Samba 4.9 (v3) (6.57 KB, patch)
2019-06-08 14:56 UTC, Andrew Bartlett 		dbagnall: review+
abartlet: review+
abartlet: ci-passed+
Details
updated advisory v3 (fix version s/4.9.10/4.9.9/) (1.77 KB, text/plain)
2019-06-13 09:02 UTC, Karolin Seeger 		kseeger: review+
abartlet: review+
dbagnall: review+
Details
Show Obsolete (8) 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 Douglas Bagnall 2019-05-02 05:35:04 UTC 
Created attachment 15116 [details]
patch adding a samba-tool subcommand to crash an rpc server.

In a couple of places in the RPC DNS server we do this:

 		z = dnsserver_find_zone(dsstate->zones, r->in.pszZone);
		if (z == NULL && request_filter == 0) {
 			return WERR_DNS_ERROR_ZONE_DOES_NOT_EXIST;
 		}
 
 		ret = dnsserver_operate_zone(dsstate, mem_ctx, z, ...);

where z can be made to be NULL by setting r->in.pszZone to a non-existent zone,
and request_filter can be made non-zero by setting r->in.dwContext to non-zero.

The combination of z == NULL && request_filter != 0 results in a NULL 
dereference in dnsserver_operate_zone(). 

The contents of r->in.pszZone and r->in.dwContext are passed over the network by
the client. As far as I can tell this function is only reached by authenticated 
users -- but the user doesn't need to have rights to perform the operation they are 
pretending to ask for.

In prefork mode there are multiple prc server workers which spring back to life after a delay, but it is very easy for an attacker to continue killing them all.

This was found via Coverity CID 1418127.
Comment 1 Douglas Bagnall 2019-05-02 05:36:51 UTC 
At first glance you might think

		if (z == NULL && request_filter == 0) {

was supposed to be

		if (z == NULL || request_filter == 0) {

but actually request_filter == 0 is the common case. It is possible that

		if (z == NULL || 
request_filter == 0) {
Comment 2 Douglas Bagnall 2019-05-02 05:40:42 UTC 
(In reply to Douglas Bagnall from comment #1)
[emacs-fingers pushed something that caused submit]

At first glance you might think

		if (z == NULL && request_filter == 0) {

was supposed to be

		if (z == NULL || request_filter == 0) {

but actually request_filter == 0 is the common case. It is possible that

		if (z == NULL || 
                    ((strncmp(z, "..", 2) == 0) && request_filter == 0) {

is right, otherwise simply 

		if (z == NULL) {
Comment 3 Andrew Bartlett 2019-05-19 22:29:39 UTC 
Created attachment 15168 [details]
first draft security advisory
Comment 4 Andreas Schneider 2019-05-20 12:16:18 UTC 
A user on #samba-technical reported:

[Friday, May 17, 2019] [11:58:55 AM CEST] <kallenp> When I add wrong DNS SRV record via RSAT DNS management console I have this problem in my syslog https://pastebin.com/TeSurBbE and this in log.samba https://pastebin.com/aLRnYjdb when I try use RSAT DNS management console. Now I cannot query DNS record from console too https://pastebin.com/78hKCj0j Please, any idea how can I fix this manually?

Maybe we should check if this is the same issue and is fixed now. Not that there is another crash.
Comment 5 Douglas Bagnall 2019-05-22 01:34:54 UTC 
Created attachment 15176 [details]
patch for master
Comment 6 Douglas Bagnall 2019-05-22 01:39:58 UTC 
(In reply to Douglas Bagnall from comment #2)

So presumably, even if something like

		if (z == NULL || 
                    ((strncmp(z, "..", 2) == 0) && request_filter == 0) {


was intended, it never worked as intended because it always segfaulted, so we can safely say that removing the request_filter condition will not cause a regression. Making it work properly is beyond the scope of a security bug.

Thus removing the condition is what attachment 15176 [details] does.
Comment 7 Aaron Haslett (dead mail address) 2019-05-29 05:17:04 UTC 
(In reply to Andreas Schneider from comment #4)
Comment 8 Aaron Haslett (dead mail address) 2019-05-29 05:53:25 UTC 
I come to you now in penance.

Issue was introduced by me in 4.9.

commit d6e111ff4212bbab6f8fdc67828afe4d1c154ac4
Author: Aaron Haslett <aaronhaslett@catalyst.net.nz>
Date:   Tue Jul 3 15:34:32 2018 +1200

    rpc dns: reset dword aging related zone properties
    
    This allows a user to set zone properties relevant to DNS record aging over RPC.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=10812
    
    Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>
    Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet@samba.org>


Relevant change in dnsserver_operate_zone is:
+               return dnsserver_db_do_reset_dword(dsstate->samdb, z,
+                                                  r->NameAndParam);

If z is NULL, we get a segfault, but a request with NULL zone name is valid if the request is for a multizone operation, indicated by a non-zero dwContext.  I didn't know that when I wrote the patch.  We don't actually support multizone operations so Douglas's patch (which never calls dnsserver_operate_zone if z is NULL) is correct, but I'll add a comment.
Comment 9 Aaron Haslett (dead mail address) 2019-05-29 05:54:56 UTC 
(In reply to Andreas Schneider from comment #4)
The issue here was introduced in 4.9, but those logs show the user is on 4.5.16-Debian, so that issue is unrelated.
Comment 10 Aaron Haslett (dead mail address) 2019-05-29 06:44:07 UTC 
Created attachment 15200 [details]
patch for master v2

Passes "make test TESTS=samba.tests.dcerpc.dnsserver" on releases 4.9.8, 4.10.4, and master.

CI 4.9.8: https://gitlab.catalyst.net.nz/samba/samba.org-security-patches/pipelines/139304

CI 4.10.4: https://gitlab.catalyst.net.nz/samba/samba.org-security-patches/pipelines/139305
Comment 11 Aaron Haslett (dead mail address) 2019-05-29 06:46:21 UTC 
Created attachment 15201 [details]
Security Advisory

updated versions and CVE
Comment 12 Joe Guo (mail address dead) 2019-05-31 05:09:30 UTC 
Created attachment 15208 [details]
Patch for master

Changes the commit message titles to include CVE number and cherry-pick -x onto master.
Comment 13 Joe Guo (mail address dead) 2019-05-31 05:12:05 UTC 
Created attachment 15209 [details]
Backport patch for v4-10-test

Changes the commit message titles to include CVE number and cherry-pick -x onto v4-10-test.
CI passed at: https://gitlab.catalyst.net.nz/samba/samba.org-security-patches/pipelines/139305
Comment 14 Joe Guo (mail address dead) 2019-05-31 05:13:31 UTC 
Created attachment 15210 [details]
Backport patch for v4-9-test

Changes the commit message titles to include CVE number and cherry-pick -x onto v4-9-test.
CI passed at: https://gitlab.catalyst.net.nz/samba/samba.org-security-patches/pipelines/139304
Comment 15 Joe Guo (mail address dead) 2019-05-31 05:17:07 UTC 
CI for master is in progress at:

https://gitlab.catalyst.net.nz/samba/samba.org-security-patches/pipelines/139681

Have to make a gitlab ci yml syntax change on top to trigger our private gitlab ci.
Comment 16 Douglas Bagnall 2019-06-02 05:39:52 UTC 
Don't we want the backports on 4-x-stable, not 4-x-test?

(Unless we know there is a scheduled bugfix release before this security release).
Comment 17 Douglas Bagnall 2019-06-08 09:03:14 UTC 
Created attachment 15229 [details]
Updated advisory (v02)

Updated advisory:

* expand $VERSIONS and CID.
* minor reformatting.
Comment 18 Andrew Bartlett 2019-06-08 14:41:00 UTC 
Created attachment 15232 [details]
patch for master (v3)

Removes the incorrect cherry-pick lines
Comment 19 Andrew Bartlett 2019-06-08 14:45:25 UTC 
Created attachment 15233 [details]
patch for Samba 4.10 (v3)

Remove incorrect cherry-pick markers and add Reviewed-by tags. 

Adding ci-passed as the patches are otherwise identical to patches tested on 4.10-stable already.
Comment 20 Andrew Bartlett 2019-06-08 14:46:35 UTC 
(In reply to Andrew Bartlett from comment #19)
The patch it is identical to is the patch in attachment 15209 [details]
Comment 21 Andrew Bartlett 2019-06-08 14:56:01 UTC 
Created attachment 15234 [details]
patch for Samba 4.9 (v3)

Adding ci-passed as it is otherwise identical to attachment 15210 [details] already tested on v4-9-stable.  

Remove incorrect cherry-pick and add Reviewed-by tags.
Comment 22 Andrew Bartlett 2019-06-08 20:16:29 UTC 
Assigning to Karolin for the security release.

The security release will be per the tracking bug 13990, currently scheduled for 19 June.
Comment 23 Karolin Seeger 2019-06-13 09:02:04 UTC 
Created attachment 15246 [details]
updated advisory v3 (fix version s/4.9.10/4.9.9/)
Comment 24 Andrew Bartlett 2019-06-13 13:02:15 UTC 
Comment on attachment 15246 [details]
updated advisory v3 (fix version s/4.9.10/4.9.9/)

Thanks, well spotted.  

Sorry about that.
Comment 25 Karolin Seeger 2019-06-19 06:57:23 UTC 
Samba 4.10.5 and 4.9.9 have been shipped to address this defect.
Comment 26 Karolin Seeger 2019-06-19 06:58:55 UTC 
Pushed to autobuild-master.
Comment 27 Karolin Seeger 2019-06-19 08:51:33 UTC 
Pushed to v4-10-test and v4-9-test.
Comment 28 Andrew Bartlett 2019-06-19 17:27:24 UTC 
Patch is in master, closing out the bug.