OPENAFS-SA-2024-002: viced: Avoid unchecked ACL in StoreACL audit log

CVE-2024-10396

Currently in SRXAFS_StoreACL, if CallPreamble() or check_acl() fail, we
will jump to Bad_StoreACL, which will pass the ACL string from the
client to osi_auditU. Since check_acl() hasn't yet checked if the given
ACL contains a NUL byte, the ACL may be an unterminated string. If
auditing is enabled, this can cause garbage to be logged to the audit
log, or cause the fileserver to crash.

To avoid this, set 'rawACL' to NULL at first, only setting it to the
actual ACL string after check_acl() has succeeded. This ensures that all
code accessing 'rawACL' is guaranteed to be using a terminated string.

This may mean that we pass a NULL AUD_ACL to osi_auditU. Our auditing
code explicitly checks for and handles handles NULL strings, so this is
fine.

FIXES 135445

Change-Id: Iecde5677805a28d55c833b135732a14fd86cc985
Reviewed-on: https://gerrit.openafs.org/15913
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Andrew Deason 2024-08-21 00:41:49 -05:00 committed by Benjamin Kaduk
parent eb8b93a971
commit c9eae1e8b2

View File

@ -3136,7 +3136,7 @@ common_StoreACL(afs_uint64 opcode,
struct in_addr logHostAddr; /* host ip holder for inet_ntoa */
struct fsstats fsstats;
char *displayACL = NULL;
char *rawACL = uncheckedACL->AFSOpaque_val;
char *rawACL = NULL;
int newOpcode = (opcode == opcode_RXAFS_StoreACL);
fsstats_StartOp(&fsstats, FS_STATS_RPCIDX_STOREACL);
@ -3148,6 +3148,7 @@ common_StoreACL(afs_uint64 opcode,
if (errorCode != 0) {
goto Bad_StoreACL;
}
rawACL = uncheckedACL->AFSOpaque_val;
/* Get ptr to client data for user Id for logging */
t_client = (struct client *)rx_GetSpecific(tcon, rxcon_client_key);