OPENAFS-SA-2024-002: verify FetchACL returned a valid string

CVE-2024-10396

Analogously to how a call to RXAFS_StoreACL() with a malformed
ACL string can cause a fileserver to perform invalid memory operations,
a malformed ACL string returned in response to a call to RXAFS_FetchACL()
can cause a client to perform invalid memory operations.

Modify all the in-tree callers of the RPC to verify that the ACL
data, which is conveyed as an XDR 'opaque' but whose contents
are actually expected to be a string, is a valid C string.  If
a zero-length opaque or one without a trailing NUL is received,
treat that as an error response from the fileserver rather than
returning success.

The Unix cache manager's pioctl handler already has logic to cope with a
zero-length reply by emitting a single NUL byte to userspace.  This
special-casing seems to have been in place from the original IBM import,
though it does so by confusingly "skipping over" a NUL byte already put
in place.  For historical compatibility, preserve that behavior rather
than treating the zero-length reply as an error as we do for the other
callers.  It seems likely that this location should treat a zero-length
reply as an error just as the other call sites do, but that can be done
as a later change.

Change-Id: Ibf685e54e7e3fca6a4caac63c961cfcfb2f4732a
Reviewed-on: https://gerrit.openafs.org/15914
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Benjamin Kaduk 2024-11-04 20:33:16 -08:00
parent c9eae1e8b2
commit 0b1ccb0dbc
3 changed files with 18 additions and 2 deletions

View File

@ -447,6 +447,10 @@ cm_IoctlGetACL(cm_ioctl_t *ioctlp, cm_user_t *userp, cm_scache_t *scp, cm_req_t
if (code)
return code;
/* Ensure the ACL is a string before trying to treat it like one. */
if (acl.AFSOpaque_len == 0 || memchr(acl.AFSOpaque_val, '\0',
acl.AFSOpaque_len) == NULL)
return CM_ERROR_INVAL;
}
/* skip over return data */
tlen = (int)strlen(ioctlp->outDatap) + 1;

View File

@ -1610,10 +1610,16 @@ DECL_PIOCTL(PGetAcl)
SHARED_LOCK, NULL));
if (code == 0) {
if (acl.AFSOpaque_len == 0)
if (acl.AFSOpaque_len == 0) {
afs_pd_skip(aout, 1); /* leave the NULL */
else
} else if (memchr(acl.AFSOpaque_val, '\0', acl.AFSOpaque_len) == NULL) {
/* Do not return an unterminated ACL string. */
code = EINVAL;
} else {
afs_pd_skip(aout, acl.AFSOpaque_len); /* Length of the ACL */
}
}
return code;
}

View File

@ -58,6 +58,12 @@ afscp_FetchACL(const struct afscp_venusfid *dir, struct AFSOpaque *acl)
code = RXAFS_FetchACL(server->conns[j], &df, acl, &dfst, &vs);
if (code >= 0)
break;
/* Zero-length or non-string ACLs are malformed. */
if (acl->AFSOpaque_len == 0 || memchr(acl->AFSOpaque_val, '\0',
acl->AFSOpaque_len) == NULL) {
code = EIO;
break;
}
}
}
if (code >= 0)