You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
241 lines
7.6 KiB
241 lines
7.6 KiB
From a4a3868fda277ddf0f174b77a859c33e4c339538 Mon Sep 17 00:00:00 2001
|
|
From: Ralph Boehme <slow@samba.org>
|
|
Date: Tue, 1 Aug 2023 12:30:00 +0200
|
|
Subject: [PATCH 1/2] CVE-2023-4091: smbtorture: test overwrite dispositions on
|
|
read-only file
|
|
|
|
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15439
|
|
|
|
Signed-off-by: Ralph Boehme <slow@samba.org>
|
|
---
|
|
selftest/knownfail.d/samba3.smb2.acls | 1 +
|
|
source4/torture/smb2/acls.c | 143 ++++++++++++++++++++++++++
|
|
2 files changed, 144 insertions(+)
|
|
create mode 100644 selftest/knownfail.d/samba3.smb2.acls
|
|
|
|
diff --git a/selftest/knownfail.d/samba3.smb2.acls b/selftest/knownfail.d/samba3.smb2.acls
|
|
new file mode 100644
|
|
index 000000000000..18df260c0e50
|
|
--- /dev/null
|
|
+++ b/selftest/knownfail.d/samba3.smb2.acls
|
|
@@ -0,0 +1 @@
|
|
+^samba3.smb2.acls.OVERWRITE_READ_ONLY_FILE
|
|
diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
|
|
index a27d4e079e67..5a892d004ea8 100644
|
|
--- a/source4/torture/smb2/acls.c
|
|
+++ b/source4/torture/smb2/acls.c
|
|
@@ -2989,6 +2989,148 @@ static bool test_mxac_not_granted(struct torture_context *tctx,
|
|
return ret;
|
|
}
|
|
|
|
+static bool test_overwrite_read_only_file(struct torture_context *tctx,
|
|
+ struct smb2_tree *tree)
|
|
+{
|
|
+ NTSTATUS status;
|
|
+ struct smb2_create c;
|
|
+ const char *fname = BASEDIR "\\test_overwrite_read_only_file.txt";
|
|
+ struct smb2_handle handle = {{0}};
|
|
+ union smb_fileinfo q;
|
|
+ union smb_setfileinfo set;
|
|
+ struct security_descriptor *sd = NULL, *sd_orig = NULL;
|
|
+ const char *owner_sid = NULL;
|
|
+ int i;
|
|
+ bool ret = true;
|
|
+
|
|
+ struct tcase {
|
|
+ int disposition;
|
|
+ const char *disposition_string;
|
|
+ NTSTATUS expected_status;
|
|
+ } tcases[] = {
|
|
+#define TCASE(d, s) { \
|
|
+ .disposition = d, \
|
|
+ .disposition_string = #d, \
|
|
+ .expected_status = s, \
|
|
+ }
|
|
+ TCASE(NTCREATEX_DISP_OPEN, NT_STATUS_OK),
|
|
+ TCASE(NTCREATEX_DISP_SUPERSEDE, NT_STATUS_ACCESS_DENIED),
|
|
+ TCASE(NTCREATEX_DISP_OVERWRITE, NT_STATUS_ACCESS_DENIED),
|
|
+ TCASE(NTCREATEX_DISP_OVERWRITE_IF, NT_STATUS_ACCESS_DENIED),
|
|
+ };
|
|
+#undef TCASE
|
|
+
|
|
+ ret = smb2_util_setup_dir(tctx, tree, BASEDIR);
|
|
+ torture_assert_goto(tctx, ret, ret, done, "smb2_util_setup_dir not ok");
|
|
+
|
|
+ c = (struct smb2_create) {
|
|
+ .in.desired_access = SEC_STD_READ_CONTROL |
|
|
+ SEC_STD_WRITE_DAC |
|
|
+ SEC_STD_WRITE_OWNER,
|
|
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
|
|
+ .in.share_access = NTCREATEX_SHARE_ACCESS_READ |
|
|
+ NTCREATEX_SHARE_ACCESS_WRITE,
|
|
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
|
|
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
|
|
+ .in.fname = fname,
|
|
+ };
|
|
+
|
|
+ status = smb2_create(tree, tctx, &c);
|
|
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
|
|
+ "smb2_create failed\n");
|
|
+ handle = c.out.file.handle;
|
|
+
|
|
+ torture_comment(tctx, "get the original sd\n");
|
|
+
|
|
+ ZERO_STRUCT(q);
|
|
+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
|
|
+ q.query_secdesc.in.file.handle = handle;
|
|
+ q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER;
|
|
+
|
|
+ status = smb2_getinfo_file(tree, tctx, &q);
|
|
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
|
|
+ "smb2_getinfo_file failed\n");
|
|
+ sd_orig = q.query_secdesc.out.sd;
|
|
+
|
|
+ owner_sid = dom_sid_string(tctx, sd_orig->owner_sid);
|
|
+
|
|
+ sd = security_descriptor_dacl_create(tctx,
|
|
+ 0, NULL, NULL,
|
|
+ owner_sid,
|
|
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
|
|
+ SEC_FILE_READ_DATA,
|
|
+ 0,
|
|
+ NULL);
|
|
+
|
|
+ ZERO_STRUCT(set);
|
|
+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
|
|
+ set.set_secdesc.in.file.handle = handle;
|
|
+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
|
|
+ set.set_secdesc.in.sd = sd;
|
|
+
|
|
+ status = smb2_setinfo_file(tree, &set);
|
|
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
|
|
+ "smb2_setinfo_file failed\n");
|
|
+
|
|
+ smb2_util_close(tree, handle);
|
|
+ ZERO_STRUCT(handle);
|
|
+
|
|
+ for (i = 0; i < ARRAY_SIZE(tcases); i++) {
|
|
+ torture_comment(tctx, "Verify open with %s dispostion\n",
|
|
+ tcases[i].disposition_string);
|
|
+
|
|
+ c = (struct smb2_create) {
|
|
+ .in.create_disposition = tcases[i].disposition,
|
|
+ .in.desired_access = SEC_FILE_READ_DATA,
|
|
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
|
|
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
|
|
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
|
|
+ .in.fname = fname,
|
|
+ };
|
|
+
|
|
+ status = smb2_create(tree, tctx, &c);
|
|
+ smb2_util_close(tree, c.out.file.handle);
|
|
+ torture_assert_ntstatus_equal_goto(
|
|
+ tctx, status, tcases[i].expected_status, ret, done,
|
|
+ "smb2_create failed\n");
|
|
+ };
|
|
+
|
|
+ torture_comment(tctx, "put back original sd\n");
|
|
+
|
|
+ c = (struct smb2_create) {
|
|
+ .in.desired_access = SEC_STD_WRITE_DAC,
|
|
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
|
|
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
|
|
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
|
|
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
|
|
+ .in.fname = fname,
|
|
+ };
|
|
+
|
|
+ status = smb2_create(tree, tctx, &c);
|
|
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
|
|
+ "smb2_create failed\n");
|
|
+ handle = c.out.file.handle;
|
|
+
|
|
+ ZERO_STRUCT(set);
|
|
+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
|
|
+ set.set_secdesc.in.file.handle = handle;
|
|
+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
|
|
+ set.set_secdesc.in.sd = sd_orig;
|
|
+
|
|
+ status = smb2_setinfo_file(tree, &set);
|
|
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
|
|
+ "smb2_setinfo_file failed\n");
|
|
+
|
|
+ smb2_util_close(tree, handle);
|
|
+ ZERO_STRUCT(handle);
|
|
+
|
|
+done:
|
|
+ smb2_util_close(tree, handle);
|
|
+ smb2_util_unlink(tree, fname);
|
|
+ smb2_deltree(tree, BASEDIR);
|
|
+ return ret;
|
|
+}
|
|
+
|
|
/*
|
|
basic testing of SMB2 ACLs
|
|
*/
|
|
@@ -3017,6 +3159,7 @@ struct torture_suite *torture_smb2_acls_init(TALLOC_CTX *ctx)
|
|
test_deny1);
|
|
torture_suite_add_1smb2_test(suite, "MXAC-NOT-GRANTED",
|
|
test_mxac_not_granted);
|
|
+ torture_suite_add_1smb2_test(suite, "OVERWRITE_READ_ONLY_FILE", test_overwrite_read_only_file);
|
|
|
|
suite->description = talloc_strdup(suite, "SMB2-ACLS tests");
|
|
|
|
--
|
|
2.41.0
|
|
|
|
|
|
From 5b5e2b1714e4a242b1cea44deff1f380620872c9 Mon Sep 17 00:00:00 2001
|
|
From: Ralph Boehme <slow@samba.org>
|
|
Date: Tue, 1 Aug 2023 13:04:36 +0200
|
|
Subject: [PATCH 2/2] CVE-2023-4091: smbd: use open_access_mask for access
|
|
check in open_file()
|
|
|
|
If the client requested FILE_OVERWRITE[_IF], we're implicitly adding
|
|
FILE_WRITE_DATA to the open_access_mask in open_file_ntcreate(), but for the
|
|
access check we're using access_mask which doesn't contain the additional
|
|
right, which means we can end up truncating a file for which the user has
|
|
only read-only access via an SD.
|
|
|
|
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15439
|
|
|
|
Signed-off-by: Ralph Boehme <slow@samba.org>
|
|
---
|
|
selftest/knownfail.d/samba3.smb2.acls | 1 -
|
|
source3/smbd/open.c | 4 ++--
|
|
2 files changed, 2 insertions(+), 3 deletions(-)
|
|
delete mode 100644 selftest/knownfail.d/samba3.smb2.acls
|
|
|
|
diff --git a/selftest/knownfail.d/samba3.smb2.acls b/selftest/knownfail.d/samba3.smb2.acls
|
|
deleted file mode 100644
|
|
index 18df260c0e50..000000000000
|
|
--- a/selftest/knownfail.d/samba3.smb2.acls
|
|
+++ /dev/null
|
|
@@ -1 +0,0 @@
|
|
-^samba3.smb2.acls.OVERWRITE_READ_ONLY_FILE
|
|
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
|
|
index 94f50becb247..0c9ddfe7c948 100644
|
|
--- a/source3/smbd/open.c
|
|
+++ b/source3/smbd/open.c
|
|
@@ -1442,7 +1442,7 @@ static NTSTATUS open_file(struct smb_request *req,
|
|
dirfsp,
|
|
fsp,
|
|
false,
|
|
- access_mask);
|
|
+ open_access_mask);
|
|
|
|
if (!NT_STATUS_IS_OK(status)) {
|
|
DBG_DEBUG("smbd_check_access_rights_fsp"
|
|
@@ -1633,7 +1633,7 @@ static NTSTATUS open_file(struct smb_request *req,
|
|
status = smbd_check_access_rights_fsp(dirfsp,
|
|
fsp,
|
|
false,
|
|
- access_mask);
|
|
+ open_access_mask);
|
|
|
|
if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) &&
|
|
posix_open &&
|
|
--
|
|
2.41.0
|
|
|