aboutsummaryrefslogtreecommitdiffstats
path: root/fs
diff options
authorQihang <q.h.hack.winter@gmail.com>2026-05-17 16:25:27 +0800
committerSteve French <stfrench@microsoft.com>2026-05-27 17:17:32 -0500
commit8b33e9007b2164bae9d4b6da860551992297b7e4 (patch)
tree49c5f1d154c5c5b967a8b83aed0e7740bb614067 /fs
parentce5930fd200947c5690678a311006079b76d1268 (diff)
downloadlinux-next-history-8b33e9007b2164bae9d4b6da860551992297b7e4.tar.gz
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>
Diffstat (limited to 'fs')
-rw-r--r--fs/smb/client/cifsacl.c196
1 files changed, 129 insertions, 67 deletions
diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 786dbbc43c5b9..42a3115359dac 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -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 @@ finalize_dacl:
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 */
generated by cgit 1.3-korg (git 2.53.0) at 2026-06-01 04:06:18 +0000