Attachment 18124 Details for Bug 15422 – Updated patch for master (v1.1)

[patch] Updated patch for master (v1.1)

bug-15422-master-v1.1.patch (text/plain), 9.41 KB, created by Jeremy Allison on 2023-09-25 23:01:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2023-09-25 23:01:40 UTC

Size: 9.41 KB

patch

obsolete

>From 4604fc1170eced4ed5073cea46f516b5b71f09da Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 25 Jul 2023 17:41:04 -0700
>Subject: [PATCH 1/3] CVE-2023-3961:s3:smbd: Catch any incoming pipe path that
> could exit socket_dir.
>
>For now, SMB_ASSERT() to exit the server. We will remove
>this once the test code is in place.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15422
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/rpc_client/local_np.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
>diff --git a/source3/rpc_client/local_np.c b/source3/rpc_client/local_np.c
>index 0e912d0e35a..dfed7e7beb6 100644
>--- a/source3/rpc_client/local_np.c
>+++ b/source3/rpc_client/local_np.c
>@@ -542,6 +542,24 @@ struct tevent_req *local_np_connect_send(
> 		return tevent_req_post(req, ev);
> 	}
> 
>+	/*
>+	 * Ensure we cannot process a path that exits
>+	 * the socket_dir.
>+	 */
>+	if (ISDOTDOT(lower_case_pipename) ||
>+	    (strchr(lower_case_pipename, '/')!=NULL))
>+	{
>+		DBG_DEBUG("attempt to connect to invalid pipe pathname %s\n",
>+			lower_case_pipename);
>+		/*
>+		 * For now, panic the server until we have
>+		 * the test code in place.
>+		 */
>+		SMB_ASSERT(false);
>+		tevent_req_error(req, ENOENT);
>+		return tevent_req_post(req, ev);
>+	}
>+
> 	state->socketpath = talloc_asprintf(
> 		state, "%s/np/%s", socket_dir, lower_case_pipename);
> 	if (tevent_req_nomem(state->socketpath, req)) {
>-- 
>2.39.2
>
>
>From 5bb01cd28e637bfc85ada6a64ad3341207d341c8 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 25 Jul 2023 17:49:21 -0700
>Subject: [PATCH 2/3] CVE-2023-3961:s3:torture: Add test SMB2-INVALID-PIPENAME
> to show we allow bad pipenames with unix separators through to the UNIX
> domain socket code.
>
>The raw SMB2-INVALID-PIPENAME test passes against Windows 2022,
>as it just returns NT_STATUS_OBJECT_NAME_NOT_FOUND.
>
>Add the knownfail.
>
>BUG:https://bugzilla.samba.org/show_bug.cgi?id=15422
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> selftest/knownfail.d/badpipename |   1 +
> source3/selftest/tests.py        |  14 ++++
> source3/torture/proto.h          |   1 +
> source3/torture/test_smb2.c      | 107 +++++++++++++++++++++++++++++++
> source3/torture/torture.c        |   4 ++
> 5 files changed, 127 insertions(+)
> create mode 100644 selftest/knownfail.d/badpipename
>
>diff --git a/selftest/knownfail.d/badpipename b/selftest/knownfail.d/badpipename
>new file mode 100644
>index 00000000000..e69715f863d
>--- /dev/null
>+++ b/selftest/knownfail.d/badpipename
>@@ -0,0 +1 @@
>+^samba3.smbtorture_s3.smb2.SMB2-INVALID-PIPENAME.smbtorture$fileserver$
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 4b71ba1a1de..b2237c8baad 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -311,6 +311,20 @@ plantestsuite("samba3.smbtorture_s3.smb2.SMB2-DFS-FILENAME-LEADING-BACKSLASH",
>                 smbtorture3,
>                 "-mSMB2"])
> 
>+# BUG: https://bugzilla.samba.org/show_bug.cgi?id=15422
>+# Prevent bad pipenames.
>+#
>+plantestsuite("samba3.smbtorture_s3.smb2.SMB2-INVALID-PIPENAME",
>+                "fileserver",
>+                [os.path.join(samba3srcdir,
>+                              "script/tests/test_smbtorture_s3.sh"),
>+                'SMB2-INVALID-PIPENAME',
>+                '//$SERVER_IP/tmp',
>+                '$USERNAME',
>+                '$PASSWORD',
>+                smbtorture3,
>+                "-mSMB2"])
>+
> #
> # SMB2-NON-DFS-SHARE needs to run against a special share non-msdfs-pathname-share
> # This is an empty non-DFS share with no links, used merely to test
>diff --git a/source3/torture/proto.h b/source3/torture/proto.h
>index 17426458330..26b51209639 100644
>--- a/source3/torture/proto.h
>+++ b/source3/torture/proto.h
>@@ -133,6 +133,7 @@ bool run_smb2_non_dfs_share(int dummy);
> bool run_smb2_dfs_share_non_dfs_path(int dummy);
> bool run_smb2_dfs_filename_leading_backslash(int dummy);
> bool run_smb2_pipe_read_async_disconnect(int dummy);
>+bool run_smb2_invalid_pipename(int dummy);
> bool run_smb1_dfs_paths(int dummy);
> bool run_smb1_dfs_search_paths(int dummy);
> bool run_smb1_dfs_operations(int dummy);
>diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c
>index ff43764bb03..96ab3ee8f54 100644
>--- a/source3/torture/test_smb2.c
>+++ b/source3/torture/test_smb2.c
>@@ -5359,3 +5359,110 @@ bool run_smb2_pipe_read_async_disconnect(int dummy)
> 	}
> 	return retval;
> }
>+
>+bool run_smb2_invalid_pipename(int dummy)
>+{
>+	struct cli_state *cli = NULL;
>+	NTSTATUS status;
>+	uint64_t fid_persistent = 0;
>+	uint64_t fid_volatile = 0;
>+	const char *unknown_pipe = "badpipe";
>+	const char *invalid_pipe = "../../../../../../../../../badpipe";
>+
>+	printf("Starting SMB2-INVALID-PIPENAME\n");
>+
>+	if (!torture_init_connection(&cli)) {
>+		return false;
>+	}
>+
>+	status = smbXcli_negprot(cli->conn,
>+				cli->timeout,
>+				PROTOCOL_SMB2_02,
>+				PROTOCOL_SMB3_11);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("smbXcli_negprot returned %s\n", nt_errstr(status));
>+		return false;
>+	}
>+
>+	status = cli_session_setup_creds(cli, torture_creds);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_session_setup returned %s\n", nt_errstr(status));
>+		return false;
>+	}
>+
>+	status = cli_tree_connect(cli, "IPC$", "?????", NULL);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_tree_connect returned %s\n", nt_errstr(status));
>+		return false;
>+	}
>+
>+	/* Try and connect to an unknown pipename. */
>+	status = smb2cli_create(cli->conn,
>+				cli->timeout,
>+				cli->smb2.session,
>+				cli->smb2.tcon,
>+				unknown_pipe,
>+				SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */
>+				SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */
>+				SEC_STD_SYNCHRONIZE|
>+					SEC_FILE_READ_DATA|
>+					SEC_FILE_WRITE_DATA|
>+					SEC_FILE_READ_ATTRIBUTE, /* desired_access, */
>+				FILE_ATTRIBUTE_NORMAL, /* file_attributes, */
>+				FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */
>+				FILE_CREATE, /* create_disposition, */
>+				0, /* create_options, */
>+				NULL, /* smb2_create_blobs *blobs */
>+				&fid_persistent,
>+				&fid_volatile,
>+				NULL, /* struct smb_create_returns * */
>+				talloc_tos(), /* mem_ctx. */
>+				NULL, /* struct smb2_create_blobs * */
>+				NULL); /* struct symlink_reparse_struct */
>+	/* We should get NT_STATUS_OBJECT_NAME_NOT_FOUND */
>+	if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
>+		printf("%s:%d smb2cli_create on name %s returned %s\n",
>+			__FILE__,
>+			__LINE__,
>+			unknown_pipe,
>+			nt_errstr(status));
>+		return false;
>+	}
>+
>+	/* Try and connect to an invalid pipename containing unix separators. */
>+	status = smb2cli_create(cli->conn,
>+				cli->timeout,
>+				cli->smb2.session,
>+				cli->smb2.tcon,
>+				invalid_pipe,
>+				SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */
>+				SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */
>+				SEC_STD_SYNCHRONIZE|
>+					SEC_FILE_READ_DATA|
>+					SEC_FILE_WRITE_DATA|
>+					SEC_FILE_READ_ATTRIBUTE, /* desired_access, */
>+				FILE_ATTRIBUTE_NORMAL, /* file_attributes, */
>+				FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */
>+				FILE_CREATE, /* create_disposition, */
>+				0, /* create_options, */
>+				NULL, /* smb2_create_blobs *blobs */
>+				&fid_persistent,
>+				&fid_volatile,
>+				NULL, /* struct smb_create_returns * */
>+				talloc_tos(), /* mem_ctx. */
>+				NULL, /* struct smb2_create_blobs * */
>+				NULL); /* struct symlink_reparse_struct */
>+	/*
>+	 * We should still get NT_STATUS_OBJECT_NAME_NOT_FOUND
>+	 * (tested against Windows 2022).
>+	 */
>+	if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
>+		printf("%s:%d smb2cli_create on name %s returned %s\n",
>+			__FILE__,
>+			__LINE__,
>+			invalid_pipe,
>+			nt_errstr(status));
>+		return false;
>+	}
>+	return true;
>+}
>diff --git a/source3/torture/torture.c b/source3/torture/torture.c
>index e80e8f7d8b6..5b63fcc56d1 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -15844,6 +15844,10 @@ static struct {
> 		.name  = "SMB2-QUOTA1",
> 		.fn    = run_smb2_quota1,
> 	},
>+	{
>+		.name  = "SMB2-INVALID-PIPENAME",
>+		.fn    = run_smb2_invalid_pipename,
>+	},
> 	{
> 		.name  = "SMB2-STREAM-ACL",
> 		.fn    = run_smb2_stream_acl,
>-- 
>2.39.2
>
>
>From db6d42e027ddcc56095d4fd455f57b13d090d096 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 25 Jul 2023 17:54:41 -0700
>Subject: [PATCH 3/3] CVE-2023-3961:s3: smbd: Remove the SMB_ASSERT() that
> crashes on bad pipenames.
>
>We correctly handle this and just return ENOENT (NT_STATUS_OBJECT_NAME_NOT_FOUND).
>
>Remove knowfail.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15422
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> selftest/knownfail.d/badpipename | 1 -
> source3/rpc_client/local_np.c    | 5 -----
> 2 files changed, 6 deletions(-)
> delete mode 100644 selftest/knownfail.d/badpipename
>
>diff --git a/selftest/knownfail.d/badpipename b/selftest/knownfail.d/badpipename
>deleted file mode 100644
>index e69715f863d..00000000000
>--- a/selftest/knownfail.d/badpipename
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba3.smbtorture_s3.smb2.SMB2-INVALID-PIPENAME.smbtorture$fileserver$
>diff --git a/source3/rpc_client/local_np.c b/source3/rpc_client/local_np.c
>index dfed7e7beb6..2e9d1b4febe 100644
>--- a/source3/rpc_client/local_np.c
>+++ b/source3/rpc_client/local_np.c
>@@ -551,11 +551,6 @@ struct tevent_req *local_np_connect_send(
> 	{
> 		DBG_DEBUG("attempt to connect to invalid pipe pathname %s\n",
> 			lower_case_pipename);
>-		/*
>-		 * For now, panic the server until we have
>-		 * the test code in place.
>-		 */
>-		SMB_ASSERT(false);
> 		tevent_req_error(req, ENOENT);
> 		return tevent_req_post(req, ev);
> 	}
>-- 
>2.39.2
>

Flags:

slow: review+
jra: ci-passed+

Actions: View

Attachments on bug 15422: 18001 | 18003 | 18050 | 18062 | 18099 | 18100 | 18101 | 18102 | 18124 | 18128 | 18151