From b46c5eb3efd300d5ad1f72ee9be42ef324038fd7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 --- 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 0b323404f06..95228d5d801 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.34.1 From 6efef2d8cb0370af3d808336462a56929e55f024 Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 --- 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 8c9ed71c6f7..914ff1902e5 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -298,6 +298,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 93d0727e936..6a23bf63a48 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -124,6 +124,7 @@ bool run_smb2_dfs_paths(int dummy); 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_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 3cb6d13789a..abc50d37b5b 100644 --- a/source3/torture/test_smb2.c +++ b/source3/torture/test_smb2.c @@ -5136,3 +5136,110 @@ bool run_smb2_dfs_filename_leading_backslash(int dummy) (void)smb2_dfs_delete(cli, dfs_filename_slash); 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 1e496e74456..22194fa4f42 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -15727,6 +15727,10 @@ static struct { .name = "SMB2-DFS-FILENAME-LEADING-BACKSLASH", .fn = run_smb2_dfs_filename_leading_backslash, }, + { + .name = "SMB2-INVALID-PIPENAME", + .fn = run_smb2_invalid_pipename, + }, { .name = "SMB1-TRUNCATED-SESSSETUP", .fn = run_smb1_truncated_sesssetup, -- 2.34.1 From 117a3a7c645f3961a35b159a0738a6f5af2ead58 Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 --- 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 95228d5d801..791ded99a47 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.34.1