OPENAFS-SA-2024-003: xdr: Ensure correct string length in xdr_string 44/15944/2
authorAndrew Deason <adeason@sinenomine.net>
Fri, 16 Oct 2020 01:30:14 +0000 (20:30 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Tue, 12 Nov 2024 18:06:12 +0000 (13:06 -0500)
commita82212ab20f0635a40c52648a52a1e9eaccc4937
tree124659cd1f923a9bd9a6847a650677f93316cfea
parent0ff2cd9e0f5656e8327c5fe47935998de3669678
OPENAFS-SA-2024-003: xdr: Ensure correct string length in xdr_string

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

Reviewed-on: https://gerrit.openafs.org/15922
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 7d0675e6c6a2f3200a3884fbe46b3ef8ef9ffd24)

Change-Id: Ieb8827474a7458ce80176b14ce87f3402aed7a86
Reviewed-on: https://gerrit.openafs.org/15944
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
src/rx/xdr.c