summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHaofeng Li <lihaofeng@kylinos.cn>2026-06-26 00:52:17 +0000
committerSteve French <stfrench@microsoft.com>2026-06-30 21:29:46 -0500
commit47f0b34f6bc98ed85bfdc293e8f3e432ec24958d (patch)
treefa0f21ed9968e9035f6c9ac36a587e979d328c16
parent60908f7ebcd9b6cde74ad5711fab0f49c7970949 (diff)
ksmbd: validate num_subauth when copying ACE in set_ntacl_dacl
set_ntacl_dacl() copies each ACE from the attacker-controlled stored security descriptor verbatim into the response DACL without checking sid.num_subauth. The ACE bytes (including an unchecked num_subauth) originate from an authenticated SMB2_SET_INFO(SecInfo=DACL) that is stored raw via ksmbd_vfs_set_sd_xattr(); parse_dacl() rejects a bad ACE with `break` rather than an error, so parse_sec_desc() still returns success and the malformed SD reaches the xattr intact. On a subsequent SMB2_QUERY_INFO(SecInfo=DACL) for an inode carrying a POSIX access ACL, build_sec_desc() -> set_ntacl_dacl() -> set_posix_acl_entries_dacl() walks the copied ACEs and reads ntace->sid.sub_auth[ntace->sid.num_subauth - 1] with num_subauth taken straight from the stored SD. Since sub_auth[] is fixed at SID_MAX_SUB_AUTHORITIES (15), a crafted num_subauth (e.g. 255) drives an out-of-bounds heap read of ~1 KB with an offset fully controlled by an authenticated client. The sibling functions already gate this field: parse_dacl() -- num_subauth == 0 || > SID_MAX_SUB_AUTHORITIES parse_sid() -- num_subauth > SID_MAX_SUB_AUTHORITIES smb_copy_sid() -- min_t(u8, num_subauth, SID_MAX_SUB_AUTHORITIES) set_ntacl_dacl() is the lone inconsistent path that omits the check. Add the same num_subauth validation in set_ntacl_dacl() before copying the ACE, matching the gate already enforced by parse_dacl(). Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn> Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn> Suggested-by: Namjae Jeon <linkinjeon@kernel.org> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
-rw-r--r--fs/smb/server/smbacl.c8
1 files changed, 7 insertions, 1 deletions
diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index fc9937cedb01..9c59c8f73b66 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -745,12 +745,18 @@ static void set_ntacl_dacl(struct mnt_idmap *idmap,
if (nt_ace_size > aces_size)
break;
+ if (ntace->sid.num_subauth == 0 ||
+ ntace->sid.num_subauth > SID_MAX_SUB_AUTHORITIES)
+ goto next_ace;
+
memcpy((char *)pndace + size, ntace, nt_ace_size);
if (check_add_overflow(size, nt_ace_size, &size))
break;
+ num_aces++;
+
+next_ace:
aces_size -= nt_ace_size;
ntace = (struct smb_ace *)((char *)ntace + nt_ace_size);
- num_aces++;
}
}