From c9eae1e8b26144063e5d1db23d47ee82c4b9ef3a Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Wed, 21 Aug 2024 00:41:49 -0500 Subject: [PATCH] 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 Tested-by: Benjamin Kaduk --- src/viced/afsfileprocs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/viced/afsfileprocs.c b/src/viced/afsfileprocs.c index 3810655a35..290df17495 100644 --- a/src/viced/afsfileprocs.c +++ b/src/viced/afsfileprocs.c @@ -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);