system/freebsd-src
system/freebsd-src
1
0
Fork 0
mirror of https://github.com/freebsd/freebsd-src synced 2024-07-24 03:37:16 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

iconv uses strlen directly on user supplied memory

Browse source 
`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>
Reviewed by:	jhb, vangyzen
MFC after:	1 week
Sponsored by:	Dell EMC
Differential Revision:	https://reviews.freebsd.org/D14521
This commit is contained in:
David Bright 2018-02-26 18:23:36 +00:00
parent fae9c380ce
commit 2b08b42bae
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=330027
1 changed files with 3 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
sys/libkern/iconv.c
View file

@ -413,11 +413,11 @@ iconv_sysctl_add(SYSCTL_HANDLER_ARGS)
return EINVAL;
if (din.ia_datalen > ICONV_CSMAXDATALEN)
return EINVAL;
if (strlen(din.ia_from) >= ICONV_CSNMAXLEN)
if (strnlen(din.ia_from, sizeof(din.ia_from)) >= ICONV_CSNMAXLEN)
return EINVAL;
if (strlen(din.ia_to) >= ICONV_CSNMAXLEN)
if (strnlen(din.ia_to, sizeof(din.ia_to)) >= ICONV_CSNMAXLEN)
return EINVAL;
if (strlen(din.ia_converter) >= ICONV_CNVNMAXLEN)
if (strnlen(din.ia_converter, sizeof(din.ia_converter)) >= ICONV_CNVNMAXLEN)
return EINVAL;
if (iconv_lookupconv(din.ia_converter, &dcp) != 0)
return EINVAL;