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 <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
(cherry picked from commit bf497819e61131cfa6469971596af3aa9bd4bb49)

diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index f3f998a9c0f28c07d981bf05d58a43df30a6c57c..eea56f6e0a82bb4ed4cadd22078f9561c63e3ce2 100644 (file)
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1115,7 +1115,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 80a932566b2889746ddddbde57ecb71e09560a1a..f6a3166719e54b4bba0557a2fdd39502132d315c 100644 (file)
--- a/source3/smbd/smb2_trans2.c
+++ b/source3/smbd/smb2_trans2.c
@@ -73,20 +73,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;