From 7e13414e8ea995d438cde3e60988225f3ab4cbcd Mon Sep 17 00:00:00 2001 From: Benjamin Kaduk Date: Mon, 4 Nov 2024 20:50:50 -0800 Subject: [PATCH] OPENAFS-SA-2024-002: verify FetchACL returned only a string CVE-2024-10396 Supplement the previous commit by additionally verifying that the returned ACL string occupies the entire XDR opaque, rejecting any values returned that have an internal NUL prior to the end of the opaque. Change-Id: Iefa3d00a9a0e25ef66b7166fe952aae0603ee3d7 Reviewed-on: https://gerrit.openafs.org/15915 Reviewed-by: Benjamin Kaduk Tested-by: Benjamin Kaduk --- src/WINNT/afsd/cm_ioctl.c | 3 +++ src/afs/afs_pioctl.c | 4 ++++ src/libafscp/afscp_acl.c | 4 ++++ 3 files changed, 11 insertions(+) diff --git a/src/WINNT/afsd/cm_ioctl.c b/src/WINNT/afsd/cm_ioctl.c index b8047b3dda..e8c31b5ac6 100644 --- a/src/WINNT/afsd/cm_ioctl.c +++ b/src/WINNT/afsd/cm_ioctl.c @@ -451,6 +451,9 @@ cm_IoctlGetACL(cm_ioctl_t *ioctlp, cm_user_t *userp, cm_scache_t *scp, cm_req_t if (acl.AFSOpaque_len == 0 || memchr(acl.AFSOpaque_val, '\0', acl.AFSOpaque_len) == NULL) return CM_ERROR_INVAL; + /* Reject "strings" with trailing data after the NUL. */ + if (strlen(acl.AFSOpaque_val) + 1 != acl.AFSOpaque_len) + return CM_ERROR_INVAL; } /* skip over return data */ tlen = (int)strlen(ioctlp->outDatap) + 1; diff --git a/src/afs/afs_pioctl.c b/src/afs/afs_pioctl.c index 17715f4b49..f8a4715555 100644 --- a/src/afs/afs_pioctl.c +++ b/src/afs/afs_pioctl.c @@ -1617,6 +1617,10 @@ DECL_PIOCTL(PGetAcl) /* Do not return an unterminated ACL string. */ code = EINVAL; + } else if (strlen(acl.AFSOpaque_val) + 1 != acl.AFSOpaque_len) { + /* Do not return an ACL string that has data beyond the trailing NUL. */ + code = EINVAL; + } else { afs_pd_skip(aout, acl.AFSOpaque_len); /* Length of the ACL */ } diff --git a/src/libafscp/afscp_acl.c b/src/libafscp/afscp_acl.c index f29be29813..5632feebf7 100644 --- a/src/libafscp/afscp_acl.c +++ b/src/libafscp/afscp_acl.c @@ -64,6 +64,10 @@ afscp_FetchACL(const struct afscp_venusfid *dir, struct AFSOpaque *acl) code = EIO; break; } + if (strlen(acl->AFSOpaque_val) + 1 != acl->AFSOpaque_len) { + code = EIO; + break; + } } } if (code >= 0)