From f4dfc2d7183f126bc4a45b5cabc78c3de020925f Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Mon, 18 Sep 2023 16:14:07 -0500 Subject: [PATCH] OPENAFS-SA-2024-002: viced: Free ACL on acl_Internalize_pr error CVE-2024-10396 Currently, we don't free 'newACL' if acl_Internalize_pr() fails. If acl_Internalize_pr() has already allocated 'newACL', then the memory associated with newACL will be leaked. This can happen if parsing the given ACL fails at any point after successfully parsing the first couple of lines in the ACL. Change acl_FreeACL() to make freeing a NULL acl a no-op, to make it easier to make sure the acl has been freed. FIXES 135445 Change-Id: I87745fa9b6285574acdd5ecb613e80fa1ea37ae8 Reviewed-on: https://gerrit.openafs.org/15909 Reviewed-by: Benjamin Kaduk Tested-by: Benjamin Kaduk --- src/libacl/aclprocs.c | 4 ++++ src/viced/afsfileprocs.c | 20 ++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/libacl/aclprocs.c b/src/libacl/aclprocs.c index d4c3f8c4c9..cd07e813c9 100644 --- a/src/libacl/aclprocs.c +++ b/src/libacl/aclprocs.c @@ -116,6 +116,10 @@ acl_FreeACL(struct acl_accessList **acl) /* Releases the access list defined by acl. Returns 0 always. */ struct freeListEntry *x; + if (*acl == NULL) { + return 0; + } + x = (struct freeListEntry *) ((char *)*acl - sizeof(struct freeListEntry *) - sizeof(int)); *acl = NULL; diff --git a/src/viced/afsfileprocs.c b/src/viced/afsfileprocs.c index 631cd34eeb..3ac5042f09 100644 --- a/src/viced/afsfileprocs.c +++ b/src/viced/afsfileprocs.c @@ -1248,16 +1248,24 @@ RXFetch_AccessList(Vnode * targetptr, Vnode * parentwhentargetnotdir, static afs_int32 RXStore_AccessList(Vnode * targetptr, struct AFSOpaque *AccessList) { - struct acl_accessList *newACL; /* PlaceHolder for new access list */ + int code; + struct acl_accessList *newACL = NULL; if (acl_Internalize_pr(hpr_NameToId, AccessList->AFSOpaque_val, &newACL) - != 0) - return (EINVAL); - if ((newACL->size + 4) > VAclSize(targetptr)) - return (E2BIG); + != 0) { + code = EINVAL; + goto done; + } + if ((newACL->size + 4) > VAclSize(targetptr)) { + code = E2BIG; + goto done; + } memcpy((char *)VVnodeACL(targetptr), (char *)newACL, (int)(newACL->size)); + code = 0; + + done: acl_FreeACL(&newACL); - return (0); + return code; } /*RXStore_AccessList */