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.

Reviewed-on: https://gerrit.openafs.org/15915
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 7e13414e8ea995d438cde3e60988225f3ab4cbcd)

Change-Id: I107f89e3d8a5c3c5cd67f6296742bfca7cace0e1
Reviewed-on: https://gerrit.openafs.org/15936
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Benjamin Kaduk 2024-11-04 20:50:50 -08:00
parent 64068705b1
commit a96a3160f5
3 changed files with 11 additions and 0 deletions

View File

@ -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', if (acl.AFSOpaque_len == 0 || memchr(acl.AFSOpaque_val, '\0',
acl.AFSOpaque_len) == NULL) acl.AFSOpaque_len) == NULL)
return CM_ERROR_INVAL; 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 */ /* skip over return data */
tlen = (int)strlen(ioctlp->outDatap) + 1; tlen = (int)strlen(ioctlp->outDatap) + 1;

View File

@ -1614,6 +1614,10 @@ DECL_PIOCTL(PGetAcl)
/* Do not return an unterminated ACL string. */ /* Do not return an unterminated ACL string. */
code = EINVAL; 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 { } else {
afs_pd_skip(aout, acl.AFSOpaque_len); /* Length of the ACL */ afs_pd_skip(aout, acl.AFSOpaque_len); /* Length of the ACL */
} }

View File

@ -64,6 +64,10 @@ afscp_FetchACL(const struct afscp_venusfid *dir, struct AFSOpaque *acl)
code = EIO; code = EIO;
break; break;
} }
if (strlen(acl->AFSOpaque_val) + 1 != acl->AFSOpaque_len) {
code = EIO;
break;
}
} }
} }
if (code >= 0) if (code >= 0)