jimzah/openafs
1
0
Fork 0
mirror of https://git.openafs.org/openafs.git synced 2025-01-18 15:00:12 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

OPENAFS-SA-2024-002: viced: Refuse ACLs without '\0' in SRXAFS_StoreACL

Browse Source 
CVE-2024-10396

Currently, the fileserver treats the ACL given in RXAFS_StoreACL as a
string, even though it is technically an AFSOpaque and could be not
NUL-terminated.

We give the ACL opaque/string to acl_Internalize_pr() to parse, which
will run off the end of the allocated buffer if the given ACL does not
contain a '\0' character. Usually this will result in a parse error
since we'll encounter garbage, but if the partially-garbage ACL
happens to parse successfully, some uninitialized data could make it
into the stored ACL.

In addition, if the given ACL is an opaque of length 0, we'll still
give the opaque pointer to acl_Internalize_pr(). In this case, the
pointer will point to &memZero, which happens to contain a NUL byte,
and so is treated like an empty string (which is not a valid ACL). But
the fact that this causes no problems is somewhat a coincidence, and
so should also be avoided.

To avoid both of these situations, just check if the given ACL string
contains a NUL byte. If it doesn't, or if it has length 0, refuse to
look at it and abort the call with EINVAL.

FIXES 135445

Change-Id: If55f72d6556bc7b1704a3848865bfb902ee9f92a
Reviewed-on: https://gerrit.openafs.org/15908
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Andrew Deason 2023-09-18 16:13:57 -05:00 committed by Benjamin Kaduk
parent 21491d9a82
commit e15decb318
1 changed files with 35 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/viced/afsfileprocs.c
View File

@ -3078,6 +3078,36 @@ printableACL(struct AFSOpaque *AccessList)
return s;
}
/**
* Check if the given ACL blob is okay to use.
*
* If the given ACL is 0-length, or doesn't contain a NUL byte, return an error
* and refuse the process the given ACL. The ACL must contain a NUL byte,
* otherwise ACL-processing code may run off the end of the buffer and process
* uninitialized memory.
*
* If there is any data beyond the NUL byte, just ignore it and return success.
* We won't look at any post-NUL data, but theoretically clients could send
* such an ACL to us, since historically it's been allowed.
*
* @param[in] AccessList The ACL blob to check
*
* @returns errno error code to abort the call with
* @retval 0 ACL is okay to use
*/
static afs_int32
check_acl(struct AFSOpaque *AccessList)
{
if (AccessList->AFSOpaque_len == 0) {
return EINVAL;
}
if (memchr(AccessList->AFSOpaque_val, '\0',
AccessList->AFSOpaque_len) == NULL) {
return EINVAL;
}
return 0;
}
static afs_int32
common_StoreACL(afs_uint64 opcode,
struct rx_call * acall, struct AFSFid * Fid,
@ -3104,6 +3134,11 @@ common_StoreACL(afs_uint64 opcode,
if ((errorCode = CallPreamble(acall, ACTIVECALL, Fid, &tcon, &thost)))
goto Bad_StoreACL;
errorCode = check_acl(AccessList);
if (errorCode != 0) {
goto Bad_StoreACL;
}
/* Get ptr to client data for user Id for logging */
t_client = (struct client *)rx_GetSpecific(tcon, rxcon_client_key);
logHostAddr.s_addr = rxr_HostOf(tcon);