cifs: validate full SID length in security descriptors

parse_sid() only verified that the fixed SID header fit in the
returned security descriptor, but did not verify that the full SID
body described by num_subauth was present.

A malicious server can return a truncated owner or group SID whose
header lies within the descriptor buffer while sub_auth[] extends
past the end of the allocation, leading to an out-of-bounds read
when the client later parses or copies that SID.

Validate the full SID body in parse_sid(), centralize owner/group SID
lookup and bounds checking in sid_from_sd(), and use that validation
in parse_sec_desc(), build_sec_desc(), and copy_sec_desc() before
sub_auth[] is accessed.

Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
This commit is contained in:
Qihang 2026-05-17 16:25:27 +08:00 committed by Steve French
parent ec457f9afe
commit 29f1005b8b

View File

@ -275,6 +275,73 @@ cifs_copy_sid(struct smb_sid *dst, const struct smb_sid *src)
return size;
}
static int parse_sid(const struct smb_sid *psid, const char *end_of_acl)
{
unsigned int sid_len;
/* SID must contain the fixed header before num_subauth is trusted. */
if (end_of_acl < (const char *)psid + CIFS_SID_BASE_SIZE) {
cifs_dbg(VFS, "ACL too small to parse SID %p\n", psid);
return -EINVAL;
}
if (psid->num_subauth > SID_MAX_SUB_AUTHORITIES) {
cifs_dbg(VFS, "SID contains too many subauthorities %u\n",
psid->num_subauth);
return -EINVAL;
}
sid_len = CIFS_SID_BASE_SIZE + psid->num_subauth * sizeof(__le32);
if (end_of_acl < (const char *)psid + sid_len) {
cifs_dbg(VFS, "ACL too small to parse SID %p\n", psid);
return -EINVAL;
}
#ifdef CONFIG_CIFS_DEBUG2
if (psid->num_subauth) {
int i;
cifs_dbg(FYI, "SID revision %d num_auth %d\n",
psid->revision, psid->num_subauth);
for (i = 0; i < psid->num_subauth; i++) {
cifs_dbg(FYI, "SID sub_auth[%d]: 0x%x\n",
i, le32_to_cpu(psid->sub_auth[i]));
}
cifs_dbg(FYI, "RID 0x%x\n",
le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]));
}
#endif
return 0;
}
static int sid_from_sd(const struct smb_ntsd *pntsd, __u32 secdesclen,
__u32 sid_offset, struct smb_sid **sid)
{
struct smb_sid *psid;
char *end_of_acl;
if (secdesclen < sizeof(struct smb_ntsd)) {
cifs_dbg(VFS, "ACL too small to parse security descriptor\n");
return -EINVAL;
}
end_of_acl = (char *)pntsd + secdesclen;
if (sid_offset < sizeof(struct smb_ntsd) ||
sid_offset > secdesclen - CIFS_SID_BASE_SIZE) {
cifs_dbg(VFS, "Server returned illegal SID offset\n");
return -EINVAL;
}
psid = (struct smb_sid *)((char *)pntsd + sid_offset);
if (parse_sid(psid, end_of_acl))
return -EINVAL;
*sid = psid;
return 0;
}
static int
id_to_sid(unsigned int cid, uint sidtype, struct smb_sid *ssid)
{
@ -515,14 +582,14 @@ exit_cifs_idmap(void)
}
/* copy ntsd, owner sid, and group sid from a security descriptor to another */
static __u32 copy_sec_desc(const struct smb_ntsd *pntsd,
struct smb_ntsd *pnntsd,
__u32 sidsoffset,
struct smb_sid *pownersid,
struct smb_sid *pgrpsid)
static int copy_sec_desc(const struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
__u32 sidsoffset, __u32 secdesclen,
__u32 *pnsecdesclen, struct smb_sid *pownersid,
struct smb_sid *pgrpsid)
{
struct smb_sid *owner_sid_ptr, *group_sid_ptr;
struct smb_sid *nowner_sid_ptr, *ngroup_sid_ptr;
int rc;
/* copy security descriptor control portion */
pnntsd->revision = pntsd->revision;
@ -533,28 +600,34 @@ static __u32 copy_sec_desc(const struct smb_ntsd *pntsd,
pnntsd->gsidoffset = cpu_to_le32(sidsoffset + sizeof(struct smb_sid));
/* copy owner sid */
if (pownersid)
if (pownersid) {
owner_sid_ptr = pownersid;
else
owner_sid_ptr = (struct smb_sid *)((char *)pntsd +
le32_to_cpu(pntsd->osidoffset));
} else {
rc = sid_from_sd(pntsd, secdesclen,
le32_to_cpu(pntsd->osidoffset), &owner_sid_ptr);
if (rc)
return rc;
}
nowner_sid_ptr = (struct smb_sid *)((char *)pnntsd + sidsoffset);
cifs_copy_sid(nowner_sid_ptr, owner_sid_ptr);
/* copy group sid */
if (pgrpsid)
if (pgrpsid) {
group_sid_ptr = pgrpsid;
else
group_sid_ptr = (struct smb_sid *)((char *)pntsd +
le32_to_cpu(pntsd->gsidoffset));
} else {
rc = sid_from_sd(pntsd, secdesclen,
le32_to_cpu(pntsd->gsidoffset), &group_sid_ptr);
if (rc)
return rc;
}
ngroup_sid_ptr = (struct smb_sid *)((char *)pnntsd + sidsoffset +
sizeof(struct smb_sid));
cifs_copy_sid(ngroup_sid_ptr, group_sid_ptr);
return sidsoffset + (2 * sizeof(struct smb_sid));
*pnsecdesclen = sidsoffset + (2 * sizeof(struct smb_sid));
return 0;
}
/*
change posix mode to reflect permissions
pmode is the existing mode (we only want to overwrite part of this
@ -1232,38 +1305,6 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl,
return 0;
}
static int parse_sid(struct smb_sid *psid, char *end_of_acl)
{
/* BB need to add parm so we can store the SID BB */
/* validate that we do not go past end of ACL - sid must be at least 8
bytes long (assuming no sub-auths - e.g. the null SID */
if (end_of_acl < (char *)psid + 8) {
cifs_dbg(VFS, "ACL too small to parse SID %p\n", psid);
return -EINVAL;
}
#ifdef CONFIG_CIFS_DEBUG2
if (psid->num_subauth) {
int i;
cifs_dbg(FYI, "SID revision %d num_auth %d\n",
psid->revision, psid->num_subauth);
for (i = 0; i < psid->num_subauth; i++) {
cifs_dbg(FYI, "SID sub_auth[%d]: 0x%x\n",
i, le32_to_cpu(psid->sub_auth[i]));
}
/* BB add length check to make sure that we do not have huge
num auths and therefore go off the end */
cifs_dbg(FYI, "RID 0x%x\n",
le32_to_cpu(psid->sub_auth[psid->num_subauth-1]));
}
#endif
return 0;
}
static bool dacl_offset_valid(unsigned int acl_len, __u32 dacloffset)
{
if (acl_len < sizeof(struct smb_acl))
@ -1284,23 +1325,25 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
int rc = 0;
struct smb_sid *owner_sid_ptr, *group_sid_ptr;
struct smb_acl *dacl_ptr; /* no need for SACL ptr */
char *end_of_acl = ((char *)pntsd) + acl_len;
__u32 dacloffset;
char *end_of_acl;
__u32 dacloffset, osidoffset, gsidoffset;
if (pntsd == NULL)
return smb_EIO(smb_eio_trace_null_pointers);
if (acl_len < (int)sizeof(struct smb_ntsd)) {
cifs_dbg(VFS, "ACL too small to parse security descriptor\n");
return -EINVAL;
}
end_of_acl = ((char *)pntsd) + acl_len;
owner_sid_ptr = (struct smb_sid *)((char *)pntsd +
le32_to_cpu(pntsd->osidoffset));
group_sid_ptr = (struct smb_sid *)((char *)pntsd +
le32_to_cpu(pntsd->gsidoffset));
osidoffset = le32_to_cpu(pntsd->osidoffset);
gsidoffset = le32_to_cpu(pntsd->gsidoffset);
dacloffset = le32_to_cpu(pntsd->dacloffset);
cifs_dbg(NOISY, "revision %d type 0x%x ooffset 0x%x goffset 0x%x sacloffset 0x%x dacloffset 0x%x\n",
pntsd->revision, pntsd->type, le32_to_cpu(pntsd->osidoffset),
le32_to_cpu(pntsd->gsidoffset),
pntsd->revision, pntsd->type, osidoffset, gsidoffset,
le32_to_cpu(pntsd->sacloffset), dacloffset);
/* cifs_dump_mem("owner_sid: ", owner_sid_ptr, 64); */
rc = parse_sid(owner_sid_ptr, end_of_acl);
rc = sid_from_sd(pntsd, acl_len, osidoffset, &owner_sid_ptr);
if (rc) {
cifs_dbg(FYI, "%s: Error %d parsing Owner SID\n", __func__, rc);
return rc;
@ -1312,9 +1355,9 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
return rc;
}
rc = parse_sid(group_sid_ptr, end_of_acl);
rc = sid_from_sd(pntsd, acl_len, gsidoffset, &group_sid_ptr);
if (rc) {
cifs_dbg(FYI, "%s: Error %d mapping Owner SID to gid\n",
cifs_dbg(FYI, "%s: Error %d parsing Group SID\n",
__func__, rc);
return rc;
}
@ -1354,8 +1397,15 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
struct smb_sid *nowner_sid_ptr = NULL, *ngroup_sid_ptr = NULL;
struct smb_acl *dacl_ptr = NULL; /* no need for SACL ptr */
struct smb_acl *ndacl_ptr = NULL; /* no need for SACL ptr */
char *end_of_acl = ((char *)pntsd) + secdesclen;
char *end_of_acl;
u16 size = 0;
__u32 osidoffset, gsidoffset;
if (secdesclen < sizeof(struct smb_ntsd)) {
cifs_dbg(VFS, "ACL too small to parse security descriptor\n");
return -EINVAL;
}
end_of_acl = ((char *)pntsd) + secdesclen;
dacloffset = le32_to_cpu(pntsd->dacloffset);
if (dacloffset) {
@ -1370,10 +1420,18 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
return rc;
}
owner_sid_ptr = (struct smb_sid *)((char *)pntsd +
le32_to_cpu(pntsd->osidoffset));
group_sid_ptr = (struct smb_sid *)((char *)pntsd +
le32_to_cpu(pntsd->gsidoffset));
osidoffset = le32_to_cpu(pntsd->osidoffset);
gsidoffset = le32_to_cpu(pntsd->gsidoffset);
rc = sid_from_sd(pntsd, secdesclen, osidoffset, &owner_sid_ptr);
if (rc) {
cifs_dbg(FYI, "%s: Error %d parsing Owner SID\n", __func__, rc);
return rc;
}
rc = sid_from_sd(pntsd, secdesclen, gsidoffset, &group_sid_ptr);
if (rc) {
cifs_dbg(FYI, "%s: Error %d parsing Group SID\n", __func__, rc);
return rc;
}
if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */
ndacloffset = sizeof(struct smb_ntsd);
@ -1389,8 +1447,10 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size);
/* copy the non-dacl portion of secdesc */
*pnsecdesclen = copy_sec_desc(pntsd, pnntsd, sidsoffset,
NULL, NULL);
rc = copy_sec_desc(pntsd, pnntsd, sidsoffset, secdesclen,
pnsecdesclen, NULL, NULL);
if (rc)
return rc;
*aclflag |= CIFS_ACL_DACL;
} else {
@ -1467,8 +1527,10 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd,
sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size);
/* copy the non-dacl portion of secdesc */
*pnsecdesclen = copy_sec_desc(pntsd, pnntsd, sidsoffset,
nowner_sid_ptr, ngroup_sid_ptr);
rc = copy_sec_desc(pntsd, pnntsd, sidsoffset, secdesclen,
pnsecdesclen, nowner_sid_ptr, ngroup_sid_ptr);
if (rc)
goto chown_chgrp_exit;
chown_chgrp_exit:
/* errors could jump here. So make sure we return soon after this */