]> CyberLeo.Net >> Repos - FreeBSD/stable/10.git/commit
MFC r330027
authordab <dab@ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f>
Mon, 5 Mar 2018 16:00:05 +0000 (16:00 +0000)
committerdab <dab@ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f>
Mon, 5 Mar 2018 16:00:05 +0000 (16:00 +0000)
commit17c456fb1afb13cca8ea2446cae2d6db33a5446d
treea3e3de9f119d478161adc998ad15c96310487683
parentbf0f136fc9d39d8e24631f0f42866d81dad23f86
MFC r330027

iconv uses strlen directly on user supplied memory

`iconv_sysctl_add` from `sys/libkern/iconv.c` incorrectly limits the
size of user strings, such that several out of bounds reads could have
been possible.

static int
iconv_sysctl_add(SYSCTL_HANDLER_ARGS)
{
struct iconv_converter_class *dcp;
struct iconv_cspair *csp;
struct iconv_add_in din;
struct iconv_add_out dout;
int error;

error = SYSCTL_IN(req, &din, sizeof(din));
if (error)
return error;
if (din.ia_version != ICONV_ADD_VER)
return EINVAL;
if (din.ia_datalen > ICONV_CSMAXDATALEN)
return EINVAL;
if (strlen(din.ia_from) >= ICONV_CSNMAXLEN)
return EINVAL;
if (strlen(din.ia_to) >= ICONV_CSNMAXLEN)
return EINVAL;
if (strlen(din.ia_converter) >= ICONV_CNVNMAXLEN)
return EINVAL;
...

Since the `din` struct is directly copied from userland, there is no
guarantee that the strings supplied will be NULL terminated. The
`strlen` calls could continue reading past the designated buffer
sizes.

Declaration of `struct iconv_add_in` is found in `sys/sys/iconv.h`:

struct iconv_add_in {
int ia_version;
char ia_converter[ICONV_CNVNMAXLEN];
char ia_to[ICONV_CSNMAXLEN];
char ia_from[ICONV_CSNMAXLEN];
int ia_datalen;
const void *ia_data;
};

Our strings are followed by the `ia_datalen` member, which is checked
before the `strlen` calls:

if (din.ia_datalen > ICONV_CSMAXDATALEN)

Since `ICONV_CSMAXDATALEN` has value `0x41000` (and is `unsigned`),
this ensures that `din.ia_datalen` contains at least 1 byte of 0, so
it is not possible to trigger a read out of bounds of the `struct`
however, this code is fragile and could introduce subtle bugs in the
future if the `struct` is ever modified.

PR: 207302
Submitted by: CTurt <cturt@hardenedbsd.org>
Reported by: CTurt <cturt@hardenedbsd.org>
Sponsored by: Dell EMC

git-svn-id: svn://svn.freebsd.org/base/stable/10@330512 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
sys/libkern/iconv.c