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-003: xdr: Ensure correct string length in xdr_string

Browse Source 
CVE-2024-10397

Currently, if a caller calls an RPC with a string output argument,
like so:

{
    char *str = NULL;
    code = RXAFS_SomeCall(&str);
    /* do something with 'str' */
    xdr_free((xdrproc_t) xdr_string, &str);
}

Normally, xdr_free causes xdr_string to call osi_free, specifying the
same size that we allocated for the string. However, since we only
have a char*, the amount of space allocated for the string is not
recorded separately, and so xdr_string calculates the size of the
buffer to free by using strlen().

This works for well-formed strings, but if we fail to decode the
payload of the string, or if our peer gave us a string with a NUL byte
in the middle of it, then strlen() may be significantly less than the
actual allocated size. And so in this case, the size given to osi_free
will be wrong.

The size given to osi_free is ignored in userspace, and for KERNEL on
many platforms like Linux and DARWIN. However, it is notably not
ignored for KERNEL on Solaris and some other less supported platforms
(HPUX, Irix, NetBSD). At least on Solaris, an incorrect size given to
osi_free can cause a system panic or possibly memory corruption.

To avoid this, change xdr_string during XDR_DECODE to make sure that
strlen() of the string always reflects the allocated size. If we fail
to decode the string's payload, replace the payload with non-NUL bytes
(fill it with 'z', an arbitrary choice). And if we do successfully
decode the payload, check if the strlen() is wrong (that is, if the
payload contains NUL '\0' bytes), and fail if so, also filling the
payload with 'z'. This is only strictly needed in KERNEL on certain
platforms, but do it everywhere so our behavior is consistent.

FIXES 135043

Change-Id: I90c419a7ef0ede247187172a182863dcb4250578
Reviewed-on: https://gerrit.openafs.org/15922
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Andrew Deason 2020-10-15 20:30:14 -05:00 committed by Benjamin Kaduk
parent c732715e4e
commit 7d0675e6c6
1 changed files with 20 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/rx/xdr.c
View File

@ -573,7 +573,26 @@ xdr_string(XDR * xdrs, char **cpp, u_int maxsize)
return (FALSE);
}
sp[size] = 0;
AFS_FALLTHROUGH;
/* Get the actual string. */
if (!xdr_opaque(xdrs, sp, size)) {
/* Make sure strlen(sp) == size, so we can calculate the correct
* size for osi_free when freeing the string. */
memset(sp, 'z', size);
return FALSE;
}
/*
* If the string contains a '\0' character, the string is invalid.
* Don't allow this, because this makes it impossible for us to pass
* the correct size to osi_free later on, when freeing the string.
*/
if (strlen(sp) != size) {
/* Make sure strlen(sp) == size. */
memset(sp, 'z', size);
return FALSE;
}
return TRUE;
case XDR_ENCODE:
return (xdr_opaque(xdrs, sp, size));