Re: [PATCH] smb: client: fix slab-out-of-bounds write in id_mode_to_cifs_acl()

[Paulo Alcantara <pc@manguebit.org>]

Dominik Maier <dmnk@google.com> writes:

> In id_mode_to_cifs_acl(), while the chown branch reserves
> sizeof(struct smb_ntsd) + 2 * sizeof(struct smb_sid) (plus
> sizeof(struct smb_acl) and the worst-case ACE space after
> commit 0ee150794c75 ("smb: client: fix heap overflow in DACL owner/group
> rewrite")), the chmod branch only adds 2 * sizeof(struct smb_ace) or
> 5 * sizeof(struct smb_ace) to secdesclen (or 1 * sizeof(struct smb_ace)
> on posix).
>
> However, the chmod branch also invokes build_sec_desc() ->
> copy_sec_desc(), which unconditionally writes the Owner SID at
> pnntsd + sidsoffset and the Group SID at
> pnntsd + sidsoffset + sizeof(struct smb_sid) (2 * 68 = 136 bytes total),
> and set_chmod_dacl() always writes a struct smb_acl header (8 bytes)
> even when the source descriptor has dacloffset == 0. When a security
> descriptor has compact or overlapping Owner/Group SIDs (or
> dacloffset == 0), calling chmod() under-allocates pnntsd and writes past
> the end of the slab buffer.
>
> Reserve sizeof(struct smb_acl) + 2 * sizeof(struct smb_sid) (plus
> sizeof(struct smb_ntsd) on the posix path) in the chmod branch of
> id_mode_to_cifs_acl().
> ...

Applied.