diff options
| author | Hem Parekh <hemparekh1596@gmail.com> | 2026-06-02 16:56:46 -0700 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2026-07-04 13:41:32 +0200 |
| commit | 988c93d3bba066d8669143e6ec30bb2be9608d53 (patch) | |
| tree | 1ba31c8f63bfc0a66ffd4f60aa49c24c8c753484 | |
| parent | c8e4e0c701d0192a2efb6df059c0f9e19678c23d (diff) | |
| download | linux-988c93d3bba066d8669143e6ec30bb2be9608d53.tar.gz linux-988c93d3bba066d8669143e6ec30bb2be9608d53.zip | |
ksmbd: fix out-of-bounds read in smb_check_perm_dacl()
commit 1ef06004ed4bd6d3ed8c840d9d1a376b66d4935b upstream.
The permission-check ACE walk in smb_check_perm_dacl() validates the ACE
header size and caps sid.num_subauth at SID_MAX_SUB_AUTHORITIES, but it
never checks that ace->size is actually large enough to contain
num_subauth sub-authorities before compare_sids() dereferences them.
CIFS_SID_BASE_SIZE covers the SID header up to but excluding the
sub_auth[] array, and offsetof(struct smb_ace, sid) is the ACE header,
so the existing guards only guarantee the 8-byte SID base, i.e. zero
sub-authorities. compare_sids() then reads ace->sid.sub_auth[i] for
i < min(local_sid->num_subauth, ace->sid.num_subauth). The local
comparison SIDs (sid_everyone, sid_unix_NFS_mode, and the id_to_sid()
result) always have at least one sub-authority, and an attacker controls
the ACE revision and authority bytes (which lie within the in-bounds SID
base), so they can match one of those SIDs and force the sub_auth read.
A crafted ACE with size == 16 and num_subauth >= 1 placed at the tail of
the security descriptor therefore causes a heap out-of-bounds read of up
to SID_MAX_SUB_AUTHORITIES * sizeof(__le32) bytes past the pntsd
allocation. The security descriptor is loaded by ksmbd_vfs_get_sd_xattr()
into a buffer sized exactly to the on-disk data (kzalloc(sd_size) in
ndr_decode_v4_ntacl()), so the read lands past the allocation. The
malformed descriptor can be stored verbatim via SMB2_SET_INFO (the DACL
is not normalised before being written to the security.NTACL xattr) and
the read fires on a subsequent SMB2_CREATE access check, making this
reachable by an authenticated client on a share that uses ACL xattrs.
Add the missing num_subauth-versus-ace_size check, mirroring the
identical guards already present in the sibling parsers parse_dacl() and
smb_inherit_dacl().
Fixes: d07b26f39246 ("ksmbd: require minimum ACE size in smb_check_perm_dacl()")
Cc: stable@vger.kernel.org
Signed-off-by: Hem Parekh <hemparekh1596@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
| -rw-r--r-- | fs/smb/server/smbacl.c | 4 |
1 files changed, 3 insertions, 1 deletions
diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c index f36ecfa94146..47c6b509cf21 100644 --- a/fs/smb/server/smbacl.c +++ b/fs/smb/server/smbacl.c @@ -1331,7 +1331,9 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, break; aces_size -= ace_size; - if (ace->sid.num_subauth > SID_MAX_SUB_AUTHORITIES) + if (ace->sid.num_subauth > SID_MAX_SUB_AUTHORITIES || + ace_size < offsetof(struct smb_ace, sid) + CIFS_SID_BASE_SIZE + + sizeof(__le32) * ace->sid.num_subauth) break; if (!compare_sids(&sid, &ace->sid) || |