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: Introduce 'rawACL' in StoreACL

Browse Source 
CVE-2024-10396

Change our StoreACL implementation to refer to the 'AccessList' argument
via a new local variable called 'rawACL'. This makes it clearer to
users that the data is a string, and makes it easier for future commits
to make sure we don't access the 'AccessList' argument in certain
situations.

Update almost all users in StoreACL to refer to 'rawACL' instead of
'AccessList'. Change the name of 'AccessList' to 'uncheckedACL' to make
sure we don't miss any users. Update our check_acl() call to use
'uncheckedACL' (and not 'rawACL'), because it must use an AFSOpaque to
check the ACL.

Change RXStore_AccessList() and printableACL() to accept a plain char*
instead of a struct AFSOpaque.

This commit should not incur any noticeable behavior change. Technically
printableACL() is changed to run strlen() on the given string, but this
should not cause any noticeable change in behavior:

This change could cause printableACL() to process less of the string
than before, if the string contains a NUL byte before the end of the
AFSOpaque buffer. But this doesn't matter, since the all of our code
after this treats the ACL as a plain string, and so doesn't look at any
data beyond the first NUL. It's not possible for printableACL() to
process more data than before, because check_acl() has already checked
that the ACL string contains a NUL byte, so we must process
AFSOpaque_len bytes or fewer.

FIXES 135445

Change-Id: I26229a39528fec788d96c77012c042c278a214c9
Reviewed-on: https://gerrit.openafs.org/15912
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:29:34 -05:00 committed by Benjamin Kaduk
parent 96ab2c6f8a
commit eb8b93a971
1 changed files with 15 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/viced/afsfileprocs.c
View File

@ -1246,12 +1246,12 @@ RXFetch_AccessList(Vnode * targetptr, Vnode * parentwhentargetnotdir,
* the target dir's vnode storage.
*/
static afs_int32
RXStore_AccessList(Vnode * targetptr, struct AFSOpaque *AccessList)
RXStore_AccessList(Vnode * targetptr, char *AccessList)
{
int code;
struct acl_accessList *newACL = NULL;
if (acl_Internalize_pr(hpr_NameToId, AccessList->AFSOpaque_val, &newACL)
if (acl_Internalize_pr(hpr_NameToId, AccessList, &newACL)
!= 0) {
code = EINVAL;
goto done;
@ -3068,20 +3068,21 @@ SRXAFS_StoreData64(struct rx_call * acall, struct AFSFid * Fid,
}
static char *
printableACL(struct AFSOpaque *AccessList)
printableACL(char *AccessList)
{
char *s;
size_t i;
size_t len = strlen(AccessList);
s = calloc(1, AccessList->AFSOpaque_len + 1);
s = calloc(1, len + 1);
if (s == NULL)
return NULL;
for (i = 0; i < AccessList->AFSOpaque_len; i++) {
if (AccessList->AFSOpaque_val[i] == '\n')
for (i = 0; i < len; i++) {
if (AccessList[i] == '\n')
s[i] = ' ';
else
s[i] = AccessList->AFSOpaque_val[i];
s[i] = AccessList[i];
}
return s;
}
@ -3119,7 +3120,7 @@ check_acl(struct AFSOpaque *AccessList)
static afs_int32
common_StoreACL(afs_uint64 opcode,
struct rx_call * acall, struct AFSFid * Fid,
struct AFSOpaque * AccessList,
struct AFSOpaque *uncheckedACL,
struct AFSFetchStatus * OutStatus, struct AFSVolSync * Sync)
{
Vnode *targetptr = 0; /* pointer to input fid */
@ -3135,6 +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;
int newOpcode = (opcode == opcode_RXAFS_StoreACL);
fsstats_StartOp(&fsstats, FS_STATS_RPCIDX_STOREACL);
@ -3142,7 +3144,7 @@ common_StoreACL(afs_uint64 opcode,
if ((errorCode = CallPreamble(acall, ACTIVECALL, Fid, &tcon, &thost)))
goto Bad_StoreACL;
errorCode = check_acl(AccessList);
errorCode = check_acl(uncheckedACL);
if (errorCode != 0) {
goto Bad_StoreACL;
}
@ -3150,12 +3152,12 @@ common_StoreACL(afs_uint64 opcode,
/* 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);
displayACL = printableACL(AccessList);
displayACL = printableACL(rawACL);
ViceLog(newOpcode ? 1 : 0,
("SAFS_StoreACL%s, Fid = %u.%u.%u, ACL=%s, Host %s:%d, Id %d\n",
newOpcode ? "" : " CVE-2018-7168",
Fid->Volume, Fid->Vnode, Fid->Unique,
displayACL == NULL ? AccessList->AFSOpaque_val : displayACL,
displayACL == NULL ? rawACL : displayACL,
inet_ntoa(logHostAddr), ntohs(rxr_PortOf(tcon)), t_client->z.ViceId));
FS_LOCK;
AFSCallStats.StoreACL++, AFSCallStats.TotalCalls++;
@ -3190,7 +3192,7 @@ common_StoreACL(afs_uint64 opcode,
}
/* Build and store the new Access List for the dir */
if ((errorCode = RXStore_AccessList(targetptr, AccessList))) {
if ((errorCode = RXStore_AccessList(targetptr, rawACL))) {
goto Bad_StoreACL;
}
@ -3222,7 +3224,7 @@ common_StoreACL(afs_uint64 opcode,
osi_auditU(acall, StoreACLEvent, errorCode,
AUD_ID, t_client ? t_client->z.ViceId : 0,
AUD_FID, Fid, AUD_ACL,
displayACL == NULL ? AccessList->AFSOpaque_val : displayACL,
displayACL == NULL ? rawACL : displayACL,
AUD_END);
free(displayACL);
return errorCode;