Skip to content

Commit

Permalink
ksmbd: validate num_aces field of smb_acl validation
Browse files Browse the repository at this point in the history 
parse_dcal() validate num_aces to allocate posix_ace_state_array.

if (num_aces > ULONG_MAX / sizeof(struct smb_ace *))

It is an incorrect validation that we can create an array of size ULONG_MAX.
smb_acl has ->size field to calculate actual number of aces in request buffer
size. Use this to check for invalid num_aces.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Signed-off-by: Namjae Jeon <[email protected]>
  • Loading branch information
@namjaejeon
namjaejeon committed Feb 12, 2025
1 parent 6f0e5a0 commit 960b6ae
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 16 deletions.
32 changes: 18 additions & 14 deletions smbacl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ void posix_state_to_acl(struct posix_acl_state *state,
pace->e_perm = state->other.allow;
}

int init_acl_state(struct posix_acl_state *state, int cnt)
int init_acl_state(struct posix_acl_state *state, u16 cnt)
{
int alloc;

Expand Down Expand Up @@ -426,7 +426,7 @@ static void parse_dacl(struct user_namespace *user_ns,
struct smb_fattr *fattr)
{
int i, ret;
int num_aces = 0;
u16 num_aces = 0;
unsigned int acl_size;
char *acl_base;
struct smb_ace **ppace;
Expand All @@ -447,16 +447,18 @@ static void parse_dacl(struct user_namespace *user_ns,

ksmbd_debug(SMB, "DACL revision %d size %d num aces %d\n",
le16_to_cpu(pdacl->revision), le16_to_cpu(pdacl->size),
le32_to_cpu(pdacl->num_aces));
le16_to_cpu(pdacl->num_aces));

acl_base = (char *)pdacl;
acl_size = sizeof(struct smb_acl);

num_aces = le32_to_cpu(pdacl->num_aces);
num_aces = le16_to_cpu(pdacl->num_aces);
if (num_aces <= 0)
return;

if (num_aces > ULONG_MAX / sizeof(struct smb_ace *))
if (num_aces > (pdacl->size - sizeof(struct smb_acl)) /
(offsetof(struct smb_ace, sid) +
offsetof(struct smb_sid, sub_auth) + sizeof(__le16)))
return;

ret = init_acl_state(&acl_state, num_aces);
Expand Down Expand Up @@ -490,6 +492,7 @@ static void parse_dacl(struct user_namespace *user_ns,
offsetof(struct smb_sid, sub_auth);

if (end_of_acl - acl_base < acl_size ||
ppace[i]->sid.num_subauth == 0 ||
ppace[i]->sid.num_subauth > SID_MAX_SUB_AUTHORITIES ||
(end_of_acl - acl_base <
acl_size + sizeof(__le32) * ppace[i]->sid.num_subauth) ||
Expand Down Expand Up @@ -646,7 +649,7 @@ static void set_posix_acl_entries_dacl(struct mnt_idmap *idmap,
static void set_posix_acl_entries_dacl(struct user_namespace *user_ns,
#endif
struct smb_ace *pndace,
struct smb_fattr *fattr, u32 *num_aces,
struct smb_fattr *fattr, u16 *num_aces,
u16 *size, u32 nt_aces_num)
{
struct posix_acl_entry *pace;
Expand Down Expand Up @@ -787,7 +790,7 @@ static void set_ntacl_dacl(struct user_namespace *user_ns,
struct smb_fattr *fattr)
{
struct smb_ace *ntace, *pndace;
int nt_num_aces = le32_to_cpu(nt_dacl->num_aces), num_aces = 0;
u16 nt_num_aces = le16_to_cpu(nt_dacl->num_aces), num_aces = 0;
unsigned short size = 0;
int i;

Expand Down Expand Up @@ -830,7 +833,7 @@ static void set_mode_dacl(struct user_namespace *user_ns,
struct smb_acl *pndacl, struct smb_fattr *fattr)
{
struct smb_ace *pace, *pndace;
u32 num_aces = 0;
u16 num_aces = 0;
u16 size = 0, ace_size = 0;
uid_t uid;
const struct smb_sid *sid;
Expand Down Expand Up @@ -890,7 +893,7 @@ static void set_mode_dacl(struct user_namespace *user_ns,
fattr->cf_mode, 0007);

out:
pndacl->num_aces = cpu_to_le32(num_aces);
pndacl->num_aces = cpu_to_le16(num_aces);
pndacl->size = cpu_to_le16(le16_to_cpu(pndacl->size) + size);
}

Expand Down Expand Up @@ -1137,7 +1140,8 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,
struct user_namespace *user_ns = mnt_user_ns(path->mnt);
#endif
int inherited_flags = 0, flags = 0, i, ace_cnt = 0, nt_size = 0, pdacl_size;
int rc = 0, num_aces, dacloffset, pntsd_type, pntsd_size, acl_len, aces_size;
int rc = 0, dacloffset, pntsd_type, pntsd_size, acl_len, aces_size;
u16 num_aces;
char *aces_base;
bool is_dir = S_ISDIR(d_inode(path->dentry)->i_mode);

Expand All @@ -1157,7 +1161,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,

parent_pdacl = (struct smb_acl *)((char *)parent_pntsd + dacloffset);
acl_len = pntsd_size - dacloffset;
num_aces = le32_to_cpu(parent_pdacl->num_aces);
num_aces = le16_to_cpu(parent_pdacl->num_aces);
pntsd_type = le16_to_cpu(parent_pntsd->type);
pdacl_size = le16_to_cpu(parent_pdacl->size);

Expand Down Expand Up @@ -1317,7 +1321,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,
pdacl = (struct smb_acl *)((char *)pntsd + le32_to_cpu(pntsd->dacloffset));
pdacl->revision = cpu_to_le16(2);
pdacl->size = cpu_to_le16(sizeof(struct smb_acl) + nt_size);
pdacl->num_aces = cpu_to_le32(ace_cnt);
pdacl->num_aces = cpu_to_le16(ace_cnt);
pace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
memcpy(pace, aces_base, nt_size);
pntsd_size += sizeof(struct smb_acl) + nt_size;
Expand Down Expand Up @@ -1411,7 +1415,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,

ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
aces_size = acl_size - sizeof(struct smb_acl);
for (i = 0; i < le32_to_cpu(pdacl->num_aces); i++) {
for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) {
if (offsetof(struct smb_ace, access_req) > aces_size)
break;
ace_size = le16_to_cpu(ace->size);
Expand All @@ -1432,7 +1436,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,

ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
aces_size = acl_size - sizeof(struct smb_acl);
for (i = 0; i < le32_to_cpu(pdacl->num_aces); i++) {
for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) {
if (offsetof(struct smb_ace, access_req) > aces_size)
break;
ace_size = le16_to_cpu(ace->size);
Expand Down
5 changes: 3 additions & 2 deletions smbacl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ struct smb_sid {
struct smb_acl {
__le16 revision; /* revision level */
__le16 size;
__le32 num_aces;
__le16 num_aces;
__le16 reserved;
} __packed;

struct smb_ace {
Expand Down Expand Up @@ -206,7 +207,7 @@ int build_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd,
#endif
struct smb_ntsd *ppntsd, int ppntsd_size, int addition_info,
__u32 *secdesclen, struct smb_fattr *fattr);
int init_acl_state(struct posix_acl_state *state, int cnt);
int init_acl_state(struct posix_acl_state *state, u16 cnt);
void free_acl_state(struct posix_acl_state *state);
void posix_state_to_acl(struct posix_acl_state *state,
struct posix_acl_entry *pace);
Expand Down

0 comments on commit 960b6ae

Please sign in to comment.