Merge tag 'samba-4.11.15' into v4-11-test

samba: tag release samba-4.11.15

6 files changed, 228 insertions, 17 deletions

diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index a46f56c3bd9..8869edcdc87 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,4 +1,98 @@
                    ===============================
+                   Release Notes for Samba 4.11.15
+                          October 29, 2020
+                   ===============================
+
+
+This is a security release in order to address the following defects:
+
+o CVE-2020-14318: Missing handle permissions check in SMB1/2/3 ChangeNotify.
+o CVE-2020-14323: Unprivileged user can crash winbind.
+o CVE-2020-14383: An authenticated user can crash the DCE/RPC DNS with easily
+		  crafted records.
+
+
+=======
+Details
+=======
+
+o  CVE-2020-14318:
+   The SMB1/2/3 protocols have a concept of "ChangeNotify", where a client can
+   request file name notification on a directory handle when a condition such as
+   "new file creation" or "file size change" or "file timestamp update" occurs.
+
+   A missing permissions check on a directory handle requesting ChangeNotify
+   meant that a client with a directory handle open only for
+   FILE_READ_ATTRIBUTES (minimal access rights) could be used to obtain change
+   notify replies from the server. These replies contain information that should
+   not be available to directory handles open for FILE_READ_ATTRIBUTE only.
+
+o  CVE-2020-14323:
+   winbind in version 3.6 and later implements a request to translate multiple
+   Windows SIDs into names in one request. This was done for performance
+   reasons: The Microsoft RPC call domain controllers offer to do this
+   translation, so it was an obvious extension to also offer this batch
+   operation on the winbind unix domain stream socket that is available to local
+   processes on the Samba server.
+
+   Due to improper input validation a hand-crafted packet can make winbind
+   perform a NULL pointer dereference and thus crash.
+
+o  CVE-2020-14383:
+   Some DNS records (such as MX and NS records) usually contain data in the
+   additional section. Samba's dnsserver RPC pipe (which is an administrative
+   interface not used in the DNS server itself) made an error in handling the
+   case where there are no records present: instead of noticing the lack of
+   records, it dereferenced uninitialised memory, causing the RPC server to
+   crash. This RPC server, which also serves protocols other than dnsserver,
+   will be restarted after a short delay, but it is easy for an authenticated
+   non-admin attacker to crash it again as soon as it returns. The Samba DNS
+   server itself will continue to operate, but many RPC services will not.
+
+For more details, please refer to the security advisories.
+
+
+Changes since 4.11.14
+---------------------
+
+o  Jeremy Allison <jra@samba.org>
+   * BUG 14434: CVE-2020-14318: s3: smbd: Ensure change notifies can't get set
+     unless the directory handle is open for SEC_DIR_LIST.
+
+o  Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
+   * BUG 12795: CVE-2020-14383: Remote crash after adding NS or MX records using
+     'samba-tool'.
+   * BUG 14472: CVE-2020-14383: Remote crash after adding MX records.
+
+o  Volker Lendecke <vl@samba.org>
+   * BUG 14436: CVE-2020-14323: winbind: Fix invalid lookupsids DoS.
+
+
+#######################################
+Reporting bugs & Development Discussion
+#######################################
+
+Please discuss this release on the samba-technical mailing list or by
+joining the #samba-technical IRC channel on irc.freenode.net.
+
+If you do report problems then please try to send high quality
+feedback. If you don't provide vital information to help us track down
+the problem then you will probably be ignored.  All bug reports should
+be filed under the Samba 4.1 and newer product in the project's Bugzilla
+database (https://bugzilla.samba.org/).
+
+
+======================================================================
+== Our Code, Our Bugs, Our Responsibility.
+== The Samba Team
+======================================================================
+
+
+Release notes for older releases follow:
+----------------------------------------
+
+
+                   ===============================
                    Release Notes for Samba 4.11.14
                           October 06, 2020
                    ===============================
@@ -53,8 +147,7 @@ database (https://bugzilla.samba.org/).
 ======================================================================
 
 
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
 
 
                    ===============================
diff --git a/source3/smbd/notify.c b/source3/smbd/notify.c
index b36a4c0003a..68553686fa2 100644
--- a/source3/smbd/notify.c
+++ b/source3/smbd/notify.c
@@ -289,6 +289,14 @@ NTSTATUS change_notify_create(struct files_struct *fsp,
 	char fullpath[len+1];
 	NTSTATUS status = NT_STATUS_NOT_IMPLEMENTED;
 
+	/*
+	 * Setting a changenotify needs READ/LIST access
+	 * on the directory handle.
+	 */
+	if (!(fsp->access_mask & SEC_DIR_LIST)) {
+		return NT_STATUS_ACCESS_DENIED;
+	}
+
 	if (fsp->notify != NULL) {
 		DEBUG(1, ("change_notify_create: fsp->notify != NULL, "
 			  "fname = %s\n", fsp->fsp_name->base_name));
diff --git a/source3/winbindd/winbindd_lookupsids.c b/source3/winbindd/winbindd_lookupsids.c
index d28b5fa9f01..a289fd86f0f 100644
--- a/source3/winbindd/winbindd_lookupsids.c
+++ b/source3/winbindd/winbindd_lookupsids.c
@@ -47,7 +47,7 @@ struct tevent_req *winbindd_lookupsids_send(TALLOC_CTX *mem_ctx,
 	DEBUG(3, ("lookupsids\n"));
 
 	if (request->extra_len == 0) {
-		tevent_req_done(req);
+		tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
 		return tevent_req_post(req, ev);
 	}
 	if (request->extra_data.data[request->extra_len-1] != '\0') {
diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
index b6389f2328a..88efc01f154 100644
--- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
+++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
@@ -1759,15 +1759,17 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
 	TALLOC_CTX *tmp_ctx;
 	char *name;
 	const char * const attrs[] = { "name", "dnsRecord", NULL };
-	struct ldb_result *res;
-	struct DNS_RPC_RECORDS_ARRAY *recs;
+	struct ldb_result *res = NULL;
+	struct DNS_RPC_RECORDS_ARRAY *recs = NULL;
 	char **add_names = NULL;
-	char *rname;
+	char *rname = NULL;
 	const char *preference_name = NULL;
 	int add_count = 0;
 	int i, ret, len;
 	WERROR status;
-	struct dns_tree *tree, *base, *node;
+	struct dns_tree *tree = NULL;
+	struct dns_tree *base = NULL;
+	struct dns_tree *node = NULL;
 
 	tmp_ctx = talloc_new(mem_ctx);
 	W_ERROR_HAVE_NO_MEMORY(tmp_ctx);
@@ -1850,15 +1852,15 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
 		}
 	}
 
-	talloc_free(res);
-	talloc_free(tree);
-	talloc_free(name);
+	TALLOC_FREE(res);
+	TALLOC_FREE(tree);
+	TALLOC_FREE(name);
 
 	/* Add any additional records */
 	if (select_flag & DNS_RPC_VIEW_ADDITIONAL_DATA) {
 		for (i=0; i<add_count; i++) {
-			struct dnsserver_zone *z2;
-
+			struct dnsserver_zone *z2 = NULL;
+			struct ldb_message *msg = NULL;
 			/* Search all the available zones for additional name */
 			for (z2 = dsstate->zones; z2; z2 = z2->next) {
 				char *encoded_name;
@@ -1870,14 +1872,15 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
 						LDB_SCOPE_ONELEVEL, attrs,
 						"(&(objectClass=dnsNode)(name=%s)(!(dNSTombstoned=TRUE)))",
 						encoded_name);
-				talloc_free(name);
+				TALLOC_FREE(name);
 				if (ret != LDB_SUCCESS) {
 					continue;
 				}
 				if (res->count == 1) {
+					msg = res->msgs[0];
 					break;
 				} else {
-					talloc_free(res);
+					TALLOC_FREE(res);
 					continue;
 				}
 			}
@@ -1890,10 +1893,10 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
 			}
 			status = dns_fill_records_array(tmp_ctx, NULL, DNS_TYPE_A,
 							select_flag, rname,
-							res->msgs[0], 0, recs,
+							msg, 0, recs,
 							NULL, NULL);
-			talloc_free(rname);
-			talloc_free(res);
+			TALLOC_FREE(rname);
+			TALLOC_FREE(res);
 			if (!W_ERROR_IS_OK(status)) {
 				talloc_free(tmp_ctx);
 				return status;
diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c
index d8aa44f5d4c..79096394130 100644
--- a/source4/torture/smb2/notify.c
+++ b/source4/torture/smb2/notify.c
@@ -2650,6 +2650,83 @@ done:
 }
 
 /*
+  Test asking for a change notify on a handle without permissions.
+*/
+
+#define BASEDIR_HPERM BASEDIR "_HPERM"
+
+static bool torture_smb2_notify_handle_permissions(
+		struct torture_context *torture,
+		struct smb2_tree *tree)
+{
+	bool ret = true;
+	NTSTATUS status;
+	union smb_notify notify;
+	union smb_open io;
+	struct smb2_handle h1 = {{0}};
+	struct smb2_request *req;
+
+	smb2_deltree(tree, BASEDIR_HPERM);
+	smb2_util_rmdir(tree, BASEDIR_HPERM);
+
+	torture_comment(torture,
+		"TESTING CHANGE NOTIFY "
+		"ON A HANDLE WITHOUT PERMISSIONS\n");
+
+	/*
+	  get a handle on the directory
+	*/
+	ZERO_STRUCT(io.smb2);
+	io.generic.level = RAW_OPEN_SMB2;
+	io.smb2.in.create_flags = 0;
+	io.smb2.in.desired_access = SEC_FILE_READ_ATTRIBUTE;
+	io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
+	io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+	io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+				NTCREATEX_SHARE_ACCESS_WRITE;
+	io.smb2.in.alloc_size = 0;
+	io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
+	io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+	io.smb2.in.security_flags = 0;
+	io.smb2.in.fname = BASEDIR_HPERM;
+
+	status = smb2_create(tree, torture, &io.smb2);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	h1 = io.smb2.out.file.handle;
+
+	/* ask for a change notify,
+	   on file or directory name changes */
+	ZERO_STRUCT(notify.smb2);
+	notify.smb2.level = RAW_NOTIFY_SMB2;
+	notify.smb2.in.buffer_size = 1000;
+	notify.smb2.in.completion_filter = FILE_NOTIFY_CHANGE_NAME;
+	notify.smb2.in.file.handle = h1;
+	notify.smb2.in.recursive = true;
+
+	req = smb2_notify_send(tree, &notify.smb2);
+	torture_assert_goto(torture,
+			req != NULL,
+			ret,
+			done,
+			"smb2_notify_send failed\n");
+
+	/*
+	 * Cancel it, we don't really want to wait.
+	 */
+	smb2_cancel(req);
+	status = smb2_notify_recv(req, torture, &notify.smb2);
+	/* Handle h1 doesn't have permissions for ChangeNotify. */
+	CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED);
+
+done:
+	if (!smb2_util_handle_empty(h1)) {
+		smb2_util_close(tree, h1);
+	}
+	smb2_deltree(tree, BASEDIR_HPERM);
+	return ret;
+}
+
+/*
    basic testing of SMB2 change notify
 */
 struct torture_suite *torture_smb2_notify_init(TALLOC_CTX *ctx)
@@ -2682,6 +2759,9 @@ struct torture_suite *torture_smb2_notify_init(TALLOC_CTX *ctx)
 				     torture_smb2_notify_rmdir3);
 	torture_suite_add_2smb2_test(suite, "rmdir4",
 				     torture_smb2_notify_rmdir4);
+	torture_suite_add_1smb2_test(suite,
+				    "handle-permissions",
+				    torture_smb2_notify_handle_permissions);
 
 	suite->description = talloc_strdup(suite, "SMB2-NOTIFY tests");
 
diff --git a/source4/torture/winbind/struct_based.c b/source4/torture/winbind/struct_based.c
index 9745b621ca9..71f248c0d61 100644
--- a/source4/torture/winbind/struct_based.c
+++ b/source4/torture/winbind/struct_based.c
@@ -1110,6 +1110,29 @@ static bool torture_winbind_struct_lookup_name_sid(struct torture_context *tortu
 	return true;
 }
 
+static bool torture_winbind_struct_lookup_sids_invalid(
+	struct torture_context *torture)
+{
+	struct winbindd_request req = {0};
+	struct winbindd_response rep = {0};
+	bool strict = torture_setting_bool(torture, "strict mode", false);
+	bool ok;
+
+	torture_comment(torture,
+			"Running WINBINDD_LOOKUP_SIDS (struct based)\n");
+
+	ok = true;
+	DO_STRUCT_REQ_REP_EXT(WINBINDD_LOOKUPSIDS, &req, &rep,
+			      NSS_STATUS_NOTFOUND,
+			      strict,
+			      ok=false,
+			      talloc_asprintf(
+				      torture,
+				      "invalid lookupsids succeeded"));
+
+	return ok;
+}
+
 struct torture_suite *torture_winbind_struct_init(TALLOC_CTX *ctx)
 {
 	struct torture_suite *suite = torture_suite_create(ctx, "struct");
@@ -1132,6 +1155,10 @@ struct torture_suite *torture_winbind_struct_init(TALLOC_CTX *ctx)
 	torture_suite_add_simple_test(suite, "getpwent", torture_winbind_struct_getpwent);
 	torture_suite_add_simple_test(suite, "endpwent", torture_winbind_struct_endpwent);
 	torture_suite_add_simple_test(suite, "lookup_name_sid", torture_winbind_struct_lookup_name_sid);
+	torture_suite_add_simple_test(
+		suite,
+		"lookup_sids_invalid",
+		torture_winbind_struct_lookup_sids_invalid);
 
 	suite->description = talloc_strdup(suite, "WINBIND - struct based protocol tests");