OPENAFS-SA-2024-003: xdr: Set _len for prealloc'd opaque/array OUT args

CVE-2024-10397

Currently, a few RPCs with arrays or opaque OUT arguments are called
with preallocated memory for the arg, but also provide a _len of 0 (or
an uninitialized _len). This makes it impossible for the xdr routine to
tell whether we have allocated enough space to actually hold the
response from the server.

To help this situation, either specify an appropriate _len for the
preallocated value (cm_IoctlGetACL, fsprobe_LWP), or don't provide a
preallocated buffer at all and let xdr allocate a buffer for us
(PGetAcl).

Note that this commit doesn't change xdr to actually check the value of
the given _len; but now a future commit can do so without breaking
callers.

FIXES 135043

Change-Id: Ieb50aaa5ae9a1bde027999ce1c668e0c99b4d82b
Reviewed-on: https://gerrit.openafs.org/15919
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Andrew Deason 2024-06-13 15:30:50 -05:00 committed by Benjamin Kaduk
parent 00a1b266af
commit b2b1110ddd
3 changed files with 6 additions and 5 deletions

View File

@ -433,7 +433,7 @@ cm_IoctlGetACL(cm_ioctl_t *ioctlp, cm_user_t *userp, cm_scache_t *scp, cm_req_t
afid.Unique = scp->fid.unique;
do {
acl.AFSOpaque_val = ioctlp->outDatap;
acl.AFSOpaque_len = 0;
acl.AFSOpaque_len = SMB_IOCTL_MAXDATA - (ioctlp->outDatap - ioctlp->outAllocp);
code = cm_ConnFromFID(&scp->fid, userp, reqp, &connp);
if (code)
continue;

View File

@ -1593,11 +1593,10 @@ DECL_PIOCTL(PGetAcl)
return ERANGE;
Fid.Vnode |= (ain->remaining << 30);
}
acl.AFSOpaque_val = aout->ptr;
memset(&acl, 0, sizeof(acl));
do {
tconn = afs_Conn(&avc->f.fid, areq, SHARED_LOCK, &rxconn);
if (tconn) {
acl.AFSOpaque_val[0] = '\0';
XSTATS_START_TIME(AFS_STATS_FS_RPCIDX_FETCHACL);
RX_AFS_GUNLOCK();
code = RXAFS_FetchACL(rxconn, &Fid, &acl, &OutStatus, &tsync);
@ -1611,7 +1610,7 @@ DECL_PIOCTL(PGetAcl)
if (code == 0) {
if (acl.AFSOpaque_len == 0) {
afs_pd_skip(aout, 1); /* leave the NULL */
code = afs_pd_putBytes(aout, "\0", 1); /* leave a single NUL byte */
} else if (memchr(acl.AFSOpaque_val, '\0', acl.AFSOpaque_len) == NULL) {
/* Do not return an unterminated ACL string. */
@ -1622,9 +1621,10 @@ DECL_PIOCTL(PGetAcl)
code = EINVAL;
} else {
afs_pd_skip(aout, acl.AFSOpaque_len); /* Length of the ACL */
code = afs_pd_putBytes(aout, acl.AFSOpaque_val, acl.AFSOpaque_len);
}
}
xdr_free((xdrproc_t) xdr_AFSOpaque, &acl);
return code;
}

View File

@ -224,6 +224,7 @@ fsprobe_LWP(void *unused)
struct ProbeViceStatistics *curr_stats; /*Current stats region */
int *curr_probeOK; /*Current probeOK field */
ViceStatistics64 stats64; /*Current stats region */
stats64.ViceStatistics64_len = STATS64_VERSION;
stats64.ViceStatistics64_val = malloc(STATS64_VERSION *
sizeof(afs_uint64));
while (1) { /*Service loop */