[PATCH] smbd doesn't check for invalid bits in access_mask

[Ralph Böhme]

Hi!

The torture test base.createx_access, in autobuild currently not run against
smbd but only against s4 ntvfs, indicates what we're missing a check for invalid
bits in the access_mask from the client.

Patch attached. Please review & push if happy. Thanks!

Bug, in case we want to backport:
<https://bugzilla.samba.org/show_bug.cgi?id=12536>

Cheerio!
-slow
-------------- next part --------------
From 0b08d182c7c6f3cfc117500b44aa3042fb94ca0f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 23 Jan 2017 17:35:51 +0100
Subject: [PATCH 1/2] selftest: also run test base.createx_access against ad_dc

Fails currently, will be made to work in the next commit.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=12536

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail        | 1 +
 source3/selftest/tests.py | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index d96e238..aa27eea 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -39,6 +39,7 @@
 ^samba3.raw.acls nfs4acl_xattr-special.inherit_creator_group\(nt4_dc\)
 ^samba3.base.delete.deltest16a
 ^samba3.base.delete.deltest17a
+^samba3.base.createx_access.createx_access\(ad_dc\)
 ^samba3.unix.whoami anonymous connection.whoami\(ad_dc\) # We need to resolve if we should be including SID_NT_WORLD and SID_NT_NETWORK in this token
 ^samba3.unix.whoami anonymous connection.whoami\(ad_member\) # smbd maps anonymous logins to domain guest in the local domain, not SID_NT_ANONYMOUS
 # these show that we still have some differences between our system
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 37cf1a4..0b5a0ce 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -284,7 +284,7 @@ plantestsuite("samba3.async_req", "nt4_dc",
 
 #smbtorture4 tests
 
-base = ["base.attr", "base.charset", "base.chkpath", "base.defer_open", "base.delaywrite", "base.delete",
+base = ["base.attr", "base.charset", "base.chkpath", "base.createx_access", "base.defer_open", "base.delaywrite", "base.delete",
         "base.deny1", "base.deny2", "base.deny3", "base.denydos", "base.dir1", "base.dir2",
         "base.disconnect", "base.fdpass", "base.lock",
         "base.mangle", "base.negnowait", "base.ntdeny1",
@@ -340,6 +340,8 @@ tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap
 for t in tests:
     if t == "base.delaywrite":
         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD -k yes --maximum-runtime=900')
+    if t == "base.createx_access":
+        plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD -k yes --maximum-runtime=900')
     elif t == "rap.sam":
         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD --option=doscharset=ISO-8859-1')
         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD --option=doscharset=ISO-8859-1')
-- 
2.7.4


From 1713b42e81c3d4e1bf60734a611629fd878b5376 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 23 Jan 2017 16:19:06 +0100
Subject: [PATCH 2/2] s3/smbd: check for invalid access_mask
 smbd_calculate_access_mask()

This makes us pass "base.createx_access".

Bug: https://bugzilla.samba.org/show_bug.cgi?id=12536

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail  | 1 -
 source3/smbd/open.c | 6 ++++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index aa27eea..d96e238 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -39,7 +39,6 @@
 ^samba3.raw.acls nfs4acl_xattr-special.inherit_creator_group\(nt4_dc\)
 ^samba3.base.delete.deltest16a
 ^samba3.base.delete.deltest17a
-^samba3.base.createx_access.createx_access\(ad_dc\)
 ^samba3.unix.whoami anonymous connection.whoami\(ad_dc\) # We need to resolve if we should be including SID_NT_WORLD and SID_NT_NETWORK in this token
 ^samba3.unix.whoami anonymous connection.whoami\(ad_member\) # smbd maps anonymous logins to domain guest in the local domain, not SID_NT_ANONYMOUS
 # these show that we still have some differences between our system
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 404a259..931d76d 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2275,6 +2275,12 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn,
 	uint32_t orig_access_mask = access_mask;
 	uint32_t rejected_share_access;
 
+	if (access_mask & SEC_MASK_INVALID) {
+		DBG_DEBUG("access_mask [%8x] contains invalid bits\n",
+			  access_mask);
+		return NT_STATUS_ACCESS_DENIED;
+	}
+
 	/*
 	 * Convert GENERIC bits to specific bits.
 	 */
-- 
2.7.4