From bf497819e61131cfa6469971596af3aa9bd4bb49 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 21 Dec 2023 10:58:09 +0100 Subject: [PATCH] smbd: fix check_any_access_fsp() for non-fsa fsps smbd_check_access_rights_fsp() requires *all* rights in access_mask to be granted by the underlying ACL, but the semantics of this function is supposed to grant access if any one of the rights in access_requested is allowed. Fix this by looping over the requested access mask. If smbd_check_access_rights_fsp() returns sucess, mask will be non-null and when assigned to access_granted, the subsequent check will pass, fail otherwise. I'm not doing an early exit on purpose because a subsequent commit adds additional security checks that are done in the subsequent code path common for fsa and non-fsa fsps. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13688 Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- source3/smbd/proto.h | 2 +- source3/smbd/smb2_trans2.c | 38 +++++++++++++++++++++++++++++--------- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index a90326b877b..d18afe3e996 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1061,7 +1061,7 @@ NTSTATUS smb_set_file_disposition_info(connection_struct *conn, struct smb_filename *smb_fname); NTSTATUS refuse_symlink_fsp(const struct files_struct *fsp); NTSTATUS check_any_access_fsp(struct files_struct *fsp, - uint32_t access_mask); + uint32_t access_requested); uint64_t smb_roundup(connection_struct *conn, uint64_t val); bool samba_private_attr_name(const char *unix_ea_name); NTSTATUS get_ea_value_fsp(TALLOC_CTX *mem_ctx, diff --git a/source3/smbd/smb2_trans2.c b/source3/smbd/smb2_trans2.c index 81fa867688d..4bfe9f8044e 100644 --- a/source3/smbd/smb2_trans2.c +++ b/source3/smbd/smb2_trans2.c @@ -75,20 +75,40 @@ NTSTATUS refuse_symlink_fsp(const files_struct *fsp) } /** - * Check that one or more of the rights in access_mask are - * allowed. Iow, access_mask can contain more then one right and + * Check that one or more of the rights in access mask are + * allowed. Iow, access_requested can contain more then one right and * it is sufficient having only one of those granted to pass. **/ NTSTATUS check_any_access_fsp(struct files_struct *fsp, - uint32_t access_mask) + uint32_t access_requested) { - if (!fsp->fsp_flags.is_fsa) { - return smbd_check_access_rights_fsp(fsp->conn->cwd_fsp, - fsp, - false, - access_mask); + uint32_t access_granted = 0; + NTSTATUS status; + + if (fsp->fsp_flags.is_fsa) { + access_granted = fsp->access_mask; + } else { + uint32_t mask = 1; + + while (mask != 0) { + if (!(mask & access_requested)) { + mask <<= 1; + continue; + } + + status = smbd_check_access_rights_fsp( + fsp->conn->cwd_fsp, + fsp, + false, + mask); + if (NT_STATUS_IS_OK(status)) { + break; + } + mask <<= 1; + } + access_granted = mask; } - if (!(fsp->access_mask & access_mask)) { + if ((access_granted & access_requested) == 0) { return NT_STATUS_ACCESS_DENIED; } return NT_STATUS_OK; -- 2.11.4.GIT