Skip to content

Commit 960b6ae

Browse files
committed
ksmbd: validate num_aces field of smb_acl validation
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 <linkinjeon@kernel.org>
1 parent 6f0e5a0 commit 960b6ae

File tree

2 files changed

+21
-16
lines changed

2 files changed

+21
-16
lines changed

smbacl.c

+18-14
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ void posix_state_to_acl(struct posix_acl_state *state,
387387
pace->e_perm = state->other.allow;
388388
}
389389

390-
int init_acl_state(struct posix_acl_state *state, int cnt)
390+
int init_acl_state(struct posix_acl_state *state, u16 cnt)
391391
{
392392
int alloc;
393393

@@ -426,7 +426,7 @@ static void parse_dacl(struct user_namespace *user_ns,
426426
struct smb_fattr *fattr)
427427
{
428428
int i, ret;
429-
int num_aces = 0;
429+
u16 num_aces = 0;
430430
unsigned int acl_size;
431431
char *acl_base;
432432
struct smb_ace **ppace;
@@ -447,16 +447,18 @@ static void parse_dacl(struct user_namespace *user_ns,
447447

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

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

455-
num_aces = le32_to_cpu(pdacl->num_aces);
455+
num_aces = le16_to_cpu(pdacl->num_aces);
456456
if (num_aces <= 0)
457457
return;
458458

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

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

492494
if (end_of_acl - acl_base < acl_size ||
495+
ppace[i]->sid.num_subauth == 0 ||
493496
ppace[i]->sid.num_subauth > SID_MAX_SUB_AUTHORITIES ||
494497
(end_of_acl - acl_base <
495498
acl_size + sizeof(__le32) * ppace[i]->sid.num_subauth) ||
@@ -646,7 +649,7 @@ static void set_posix_acl_entries_dacl(struct mnt_idmap *idmap,
646649
static void set_posix_acl_entries_dacl(struct user_namespace *user_ns,
647650
#endif
648651
struct smb_ace *pndace,
649-
struct smb_fattr *fattr, u32 *num_aces,
652+
struct smb_fattr *fattr, u16 *num_aces,
650653
u16 *size, u32 nt_aces_num)
651654
{
652655
struct posix_acl_entry *pace;
@@ -787,7 +790,7 @@ static void set_ntacl_dacl(struct user_namespace *user_ns,
787790
struct smb_fattr *fattr)
788791
{
789792
struct smb_ace *ntace, *pndace;
790-
int nt_num_aces = le32_to_cpu(nt_dacl->num_aces), num_aces = 0;
793+
u16 nt_num_aces = le16_to_cpu(nt_dacl->num_aces), num_aces = 0;
791794
unsigned short size = 0;
792795
int i;
793796

@@ -830,7 +833,7 @@ static void set_mode_dacl(struct user_namespace *user_ns,
830833
struct smb_acl *pndacl, struct smb_fattr *fattr)
831834
{
832835
struct smb_ace *pace, *pndace;
833-
u32 num_aces = 0;
836+
u16 num_aces = 0;
834837
u16 size = 0, ace_size = 0;
835838
uid_t uid;
836839
const struct smb_sid *sid;
@@ -890,7 +893,7 @@ static void set_mode_dacl(struct user_namespace *user_ns,
890893
fattr->cf_mode, 0007);
891894

892895
out:
893-
pndacl->num_aces = cpu_to_le32(num_aces);
896+
pndacl->num_aces = cpu_to_le16(num_aces);
894897
pndacl->size = cpu_to_le16(le16_to_cpu(pndacl->size) + size);
895898
}
896899

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

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

11581162
parent_pdacl = (struct smb_acl *)((char *)parent_pntsd + dacloffset);
11591163
acl_len = pntsd_size - dacloffset;
1160-
num_aces = le32_to_cpu(parent_pdacl->num_aces);
1164+
num_aces = le16_to_cpu(parent_pdacl->num_aces);
11611165
pntsd_type = le16_to_cpu(parent_pntsd->type);
11621166
pdacl_size = le16_to_cpu(parent_pdacl->size);
11631167

@@ -1317,7 +1321,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn,
13171321
pdacl = (struct smb_acl *)((char *)pntsd + le32_to_cpu(pntsd->dacloffset));
13181322
pdacl->revision = cpu_to_le16(2);
13191323
pdacl->size = cpu_to_le16(sizeof(struct smb_acl) + nt_size);
1320-
pdacl->num_aces = cpu_to_le32(ace_cnt);
1324+
pdacl->num_aces = cpu_to_le16(ace_cnt);
13211325
pace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
13221326
memcpy(pace, aces_base, nt_size);
13231327
pntsd_size += sizeof(struct smb_acl) + nt_size;
@@ -1411,7 +1415,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
14111415

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

14331437
ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl));
14341438
aces_size = acl_size - sizeof(struct smb_acl);
1435-
for (i = 0; i < le32_to_cpu(pdacl->num_aces); i++) {
1439+
for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) {
14361440
if (offsetof(struct smb_ace, access_req) > aces_size)
14371441
break;
14381442
ace_size = le16_to_cpu(ace->size);

smbacl.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ struct smb_sid {
140140
struct smb_acl {
141141
__le16 revision; /* revision level */
142142
__le16 size;
143-
__le32 num_aces;
143+
__le16 num_aces;
144+
__le16 reserved;
144145
} __packed;
145146

146147
struct smb_ace {
@@ -206,7 +207,7 @@ int build_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd,
206207
#endif
207208
struct smb_ntsd *ppntsd, int ppntsd_size, int addition_info,
208209
__u32 *secdesclen, struct smb_fattr *fattr);
209-
int init_acl_state(struct posix_acl_state *state, int cnt);
210+
int init_acl_state(struct posix_acl_state *state, u16 cnt);
210211
void free_acl_state(struct posix_acl_state *state);
211212
void posix_state_to_acl(struct posix_acl_state *state,
212213
struct posix_acl_entry *pace);

0 commit comments

Comments
 (0)