Attachment 223071 Details for Bug 254111 – Backport 4.11 CVE-fixes to 4.10

Backport 4.11 CVE-fixes to 4.10

patch-CVE-2020-14318-14323-14383 (text/plain), 13.71 KB, created by Mikhail Teterin on 2021-03-07 19:37:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mikhail Teterin

Created: 2021-03-07 19:37:14 UTC

Size: 13.71 KB

patch

obsolete

>This is almost verbatim from Samba's patch for 4.11:
>
>	https://www.samba.org/samba/ftp/patches/security/samba-4.11.14-security-2020-10-29.patch
>
>except for one hunk, which had to be fixed for 4.10
>
>	-mi
>
>From a5da8919303ea99937c0d3b536f964b7f1addda7 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 10 Jul 2020 15:09:33 -0700
>Subject: [PATCH 1/6] s4: torture: Add smb2.notify.handle-permissions test.
>
>Add knownfail entry.
>
>CVE-2020-14318
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14434
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> .../smb2_notify_handle_permissions            |  2 +
> source4/torture/smb2/notify.c                 | 80 +++++++++++++++++++
> 2 files changed, 82 insertions(+)
> create mode 100644 selftest/knownfail.d/smb2_notify_handle_permissions
>
>diff --git selftest/knownfail.d/smb2_notify_handle_permissions b/selftest/knownfail.d/smb2_notify_handle_permissions
>new file mode 100644
>index 00000000000..c0ec8fc8153
>--- /dev/null
>+++ selftest/knownfail.d/smb2_notify_handle_permissions
>@@ -0,0 +1,2 @@
>+^samba3.smb2.notify.handle-permissions
>+
>diff --git source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c
>index d8aa44f5d4c..79096394130 100644
>--- source4/torture/smb2/notify.c
>+++ source4/torture/smb2/notify.c
>@@ -2649,6 +2649,83 @@ done:
> 	return ok;
> }
> 
>+/*
>+  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
> */
>@@ -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");
> 
>-- 
>2.17.1
>
>
>From c300a85848350635e7ddd8129b31c4d439dc0f8a Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 7 Jul 2020 18:25:23 -0700
>Subject: [PATCH 2/6] s3: smbd: Ensure change notifies can't get set unless the
> directory handle is open for SEC_DIR_LIST.
>
>Remove knownfail entry.
>
>CVE-2020-14318
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14434
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> selftest/knownfail.d/smb2_notify_handle_permissions | 2 --
> source3/smbd/notify.c                               | 8 ++++++++
> 2 files changed, 8 insertions(+), 2 deletions(-)
> delete mode 100644 selftest/knownfail.d/smb2_notify_handle_permissions
>
>diff --git selftest/knownfail.d/smb2_notify_handle_permissions b/selftest/knownfail.d/smb2_notify_handle_permissions
>deleted file mode 100644
>index c0ec8fc8153..00000000000
>--- selftest/knownfail.d/smb2_notify_handle_permissions
>+++ /dev/null
>@@ -1,2 +0,0 @@
>-^samba3.smb2.notify.handle-permissions
>-
>diff --git source3/smbd/notify.c b/source3/smbd/notify.c
>index b36a4c0003a..68553686fa2 100644
>--- source3/smbd/notify.c
>+++ 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));
>-- 
>2.17.1
>
>
>From e6fe5b4d64a8e1a03e1aaebafd97f313b3c94342 Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Thu, 9 Jul 2020 21:49:25 +0200
>Subject: [PATCH 3/6] CVE-2020-14323 winbind: Fix invalid lookupsids DoS
>
>A lookupsids request without extra_data will lead to "state->domain==NULL",
>which makes winbindd_lookupsids_recv trying to dereference it.
>
>Reported by Bas Alberts of the GitHub Security Lab Team as GHSL-2020-134
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=14436
>Signed-off-by: Volker Lendecke <vl@samba.org>
>---
> source3/winbindd/winbindd_lookupsids.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git source3/winbindd/winbindd_lookupsids.c b/source3/winbindd/winbindd_lookupsids.c
>index d28b5fa9f01..a289fd86f0f 100644
>--- source3/winbindd/winbindd_lookupsids.c
>+++ 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') {
>-- 
>2.17.1
>
>
>From 6093b2d815a00a577036fa001b47d7fc5514ad2c Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <vl@samba.org>
>Date: Thu, 9 Jul 2020 21:48:57 +0200
>Subject: [PATCH 4/6] CVE-2020-14323 torture4: Add a simple test for invalid
> lookup_sids winbind call
>
>We can't add this test before the fix, add it to knownfail and have the fix
>remove the knownfail entry again. As this crashes winbind, many tests after
>this one will fail.
>
>Reported by Bas Alberts of the GitHub Security Lab Team as GHSL-2020-134
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=14436
>Signed-off-by: Volker Lendecke <vl@samba.org>
>---
> source4/torture/winbind/struct_based.c | 27 ++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
>diff --git source4/torture/winbind/struct_based.c b/source4/torture/winbind/struct_based.c
>index 9745b621ca9..71f248c0d61 100644
>--- source4/torture/winbind/struct_based.c
>+++ 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");
> 
>-- 
>2.17.1
>
>
>From 2632e8ebae826a7305fe7d3948ee28b77d2ffbc0 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 21 Aug 2020 17:10:22 +1200
>Subject: [PATCH 5/6] CVE-2020-14383: s4/dns: Ensure variable initialization
> with NULL.
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Based on patches from Francis Brosnan BlÃ¡zquez <francis@aspl.es>
>and Jeremy Allison <jra@samba.org>
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14472
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12795
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(based on commit 7afe449e7201be92bed8e53cbb37b74af720ef4e)
>---
> .../rpc_server/dnsserver/dcerpc_dnsserver.c   | 24 ++++++++++---------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
>diff --git source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
>index b6389f2328a..ec610168266 100644
>--- source4/rpc_server/dnsserver/dcerpc_dnsserver.c
>+++ 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,9 +1852,9 @@ 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) {
>@@ -1870,14 +1872,14 @@ 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) {
> 					break;
> 				} else {
>-					talloc_free(res);
>+					TALLOC_FREE(res);
> 					continue;
> 				}
> 			}
>@@ -1892,5 +1894,5 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
> 							select_flag, rname,
> 							res->msgs[0], 0, recs,
> 							NULL, NULL);
>-			talloc_free(rname);
>-			talloc_free(res);
>+			TALLOC_FREE(rname);
>+			TALLOC_FREE(res);
>-- 
>2.17.1
>
>
>From 8e09649351e9e8143b4bd0b76bcbd2cfb4d2f281 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 21 Aug 2020 17:23:17 +1200
>Subject: [PATCH 6/6] CVE-2020-14383: s4/dns: do not crash when additional data
> not found
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Found by Francis Brosnan BlÃ¡zquez <francis@aspl.es>.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14472
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12795
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
>Autobuild-Date(master): Mon Aug 24 00:21:41 UTC 2020 on sn-devel-184
>
>(based on commit df98e7db04c901259dd089e20cd557bdbdeaf379)
>---
> source4/rpc_server/dnsserver/dcerpc_dnsserver.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
>index ec610168266..88efc01f154 100644
>--- source4/rpc_server/dnsserver/dcerpc_dnsserver.c
>+++ source4/rpc_server/dnsserver/dcerpc_dnsserver.c
>@@ -1859,8 +1859,8 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
> 	/* 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;
>@@ -1877,6 +1877,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
> 					continue;
> 				}
> 				if (res->count == 1) {
>+					msg = res->msgs[0];
> 					break;
> 				} else {
> 					TALLOC_FREE(res);
>@@ -1888,6 +1889,6 @@ static WERROR dnsserver_enumerate_record
> 			status = dns_fill_records_array(tmp_ctx, NULL, DNS_TYPE_A,
> 							select_flag, rname,
>-							res->msgs[0], 0, recs,
>+							msg, 0, recs,
> 							NULL, NULL);
> 		}
> 	}
>-- 
>2.17.1

Actions: View

Attachments on bug 254111: 223071