Re: [PATCH v2 1/2] ksmbd: bound smb_check_perm_dacl() ACE walks by DACL size
ChenXiaoSong <[email protected]>
| Newsgroups | org.kernel.vger.linux-cifs,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Reviewed-by: ChenXiaoSong <[email protected]> On 8/12/26 18:39, Hang Nan wrote: > smb_check_perm_dacl() validates that the DACL fits inside the NT > security descriptor, but then bounds its two ACE walks by the > remaining NTSD length (acl_size) rather than the DACL's declared > size (pdacl_size). > > When pdacl->size is smaller than the trailing NTSD buffer, bytes > after the declared DACL boundary - still inside the stored security > descriptor - are parsed as ACEs during access checks. A crafted > DACL can place an access-granting ACE beyond pdacl->size, and the > current code accepts it during SMB2_CREATE access validation, while > parse_dacl() and smb_inherit_dacl() stop at pdacl_size. > > Bound both ACE walks by pdacl_size to match the DACL boundary > semantics used elsewhere in the server. > > Validation (KUnit, UML, x86_64, KASAN; first KUnit tests for > fs/smb/server, see follow-up patch): > - ksmbd_dacl_walk_must_stop_at_declared_size (semantic harness): > with the current code the ACE placed after pdacl->size is > selected and access is granted; with the fix the same sample > is denied (EACCES). > - ksmbd_smb_check_perm_dacl_boundary (drives the real function): > passes with the fix; fails with the fix reverted (rc == 0). > - ksmbd-for-next-next (rebased, base e9d76059ff03), x86_64 > > Fixes: 8f0541186e9a ("ksmbd: fix heap-based overflow in set_ntacl_dacl()") > Signed-off-by: Hang Nan<[email protected]> -- ChenXiaoSong <[email protected]> Chinese Homepage: https://chenxiaosong.com English Homepage: https://chenxiaosong.com/en