Bug 206761

There is a stack overflow in the `sysctl_kern_binmisc` code path from `sys/kern/imgact_binmisc.c`:

	switch(arg2) {
	case IBC_ADD:
		/* Add an entry. Limited to IBE_MAX_ENTRIES. */
		error = SYSCTL_IN(req, &xbe, sizeof(xbe));
		if (error)
			return (error);
		if (IBE_VERSION != xbe.xbe_version)
			return (EINVAL);
		if (interp_list_entry_count == IBE_MAX_ENTRIES)
			return (ENOSPC);
		error = imgact_binmisc_add_entry(&xbe);
		break;

Notice that contents of `xbe` are user controlled, with just a check on the `xbe_version` member before calling `imgact_binmisc_add_entry`:

static int
imgact_binmisc_add_entry(ximgact_binmisc_entry_t *xbe)
{
	imgact_binmisc_entry_t *ibe;
	char *p;

	if (xbe->xbe_msize > IBE_MAGIC_MAX)
		return (EINVAL);

	for(p = xbe->xbe_name; *p != 0; p++)
		if (!isascii((int)*p))
			return (EINVAL);

	for(p = xbe->xbe_interpreter; *p != 0; p++)
		if (!isascii((int)*p))
			return (EINVAL);

Notice that already 2 out of bounds reads are possible. Since these strings come from userland, there is no guarantee that they are NULL truncated. Limiting the contentents of `sb_interpreter` to ASCII characters only is a minor annoyance, but let's continue:

	/* Make sure we don't have any invalid #'s. */
	p = xbe->xbe_interpreter;
	while (1) {
		p = strchr(p, '#');
		if (!p)
			break;

		p++;
		switch(*p) {
		case ISM_POUND:
			/* "##" */
			p++;
			break;

		case ISM_OLD_ARGV0:
			/* "#a" */
			p++;
			break;

		case 0:
		default:
			/* Anything besides the above is invalid. */
			return (EINVAL);
		}
	}

	sx_xlock(&interp_list_sx);
	if (imgact_binmisc_find_entry(xbe->xbe_name) != NULL) {
		sx_xunlock(&interp_list_sx);
		return (EEXIST);
	}

	/* Preallocate a new entry. */
	ibe = imgact_binmisc_new_entry(xbe);
	if (!ibe)
		return (ENOMEM);

	SLIST_INSERT_HEAD(&interpreter_list, ibe, link);
	interp_list_entry_count++;
	sx_xunlock(&interp_list_sx);

	return (0);
}

Basically, some more checks on the contents, but nothing limiting the size. Moving onto the `imgact_binmisc_new_entry` call:

static imgact_binmisc_entry_t *
imgact_binmisc_new_entry(ximgact_binmisc_entry_t *xbe)
{
	imgact_binmisc_entry_t *ibe = NULL;
	size_t namesz = min(strlen(xbe->xbe_name) + 1, IBE_NAME_MAX);

	ibe = malloc(sizeof(*ibe), M_BINMISC, M_WAITOK|M_ZERO);

	ibe->ibe_name = malloc(namesz, M_BINMISC, M_WAITOK|M_ZERO);
	strlcpy(ibe->ibe_name, xbe->xbe_name, namesz);

	imgact_binmisc_populate_interp(xbe->xbe_interpreter, ibe);

The important thing here is that `xbe->xbe_name` is limited to `IBE_NAME_MAX`, but there is no check on the size of `xbe->xbe_interpreter` string, it is passed directly to `imgact_binmisc_populate_interp`:

static void
imgact_binmisc_populate_interp(char *str, imgact_binmisc_entry_t *ibe)
{
	uint32_t len = 0, argc = 1;
	char t[IBE_INTERP_LEN_MAX];
	char *sp, *tp;

	memset(t, 0, sizeof(t));

	/*
	 * Normalize interpreter string. Replace white space between args with
	 * single space.
	 */
	sp = str; tp = t;
	while (*sp != '\0') {
		if (*sp == ' ' || *sp == '\t') {
			if (++len > IBE_INTERP_LEN_MAX)
				break;
			*tp++ = ' ';
			argc++;
			while (*sp == ' ' || *sp == '\t')
				sp++;
			continue;
		} else {
			*tp++ = *sp++;
			len++;
		}
	}
	*tp = '\0';
	len++;

	ibe->ibe_interpreter = malloc(len, M_BINMISC, M_WAITOK|M_ZERO);

	/* Populate all the ibe fields for the interpreter. */
	memcpy(ibe->ibe_interpreter, t, len);
	ibe->ibe_interp_argcnt = argc;
	ibe->ibe_interp_length = len;
}

Clearly, it is not safe to use this function on a user supplied string!

The characters of the input string (`str`) are iterated over without any check on size:

    while (*sp != '\0') {

There is a check on the `len` value, but only when the string reaches whitespace:

		if (*sp == ' ' || *sp == '\t') {
			if (++len > IBE_INTERP_LEN_MAX)
				break;

If the string consists of no whitespace we can increase `tp`, `sp`, and `len` freely (beyond the `IBE_INTERP_LEN_MAX` limit):

		} else {
			*tp++ = *sp++;
			len++;
		}

We then have a write from `tp`:

    *tp = '\0';

And also a `malloc` and `copyin` with `len`:

	ibe->ibe_interpreter = malloc(len, M_BINMISC, M_WAITOK|M_ZERO);

	/* Populate all the ibe fields for the interpreter. */
	memcpy(ibe->ibe_interpreter, t, len);

Remember that `t` is a fixed stack buffer:

    char t[IBE_INTERP_LEN_MAX];

So, when `len` exceeds `IBE_INTERP_LEN_MAX`, we have an out of bounds stack read here, and an out of bounds stack write from `*tp = '\0'`.

But the most exploitable thing is the `*tp++ = *sp++;` copy; we can use it write past the `t` buffer on the stack.

Here's the `ximgact_binmisc_entry` struct from `/sys/sys/imgact_binmisc.h`:

typedef struct ximgact_binmisc_entry {
	uint32_t xbe_version;	/* Struct version(IBE_VERSION) */
	uint32_t xbe_flags;	/* Entry flags (IBF_*) */
	uint32_t xbe_moffset;	/* Magic offset in header */
	uint32_t xbe_msize;	/* Magic size */
	uint32_t spare[3];	/* Spare fields for future use */
	char xbe_name[IBE_NAME_MAX];	/* Unique interpreter name */
	char xbe_interpreter[IBE_INTERP_LEN_MAX]; /* Interpreter path + args */
	uint8_t xbe_magic[IBE_MAGIC_MAX]; /* Header Magic */
	uint8_t xbe_mask[IBE_MAGIC_MAX]; /* Magic Mask */
} ximgact_binmisc_entry_t;

If we make `xbe_interpreter` non-terminated, the following fields, `xbe_magic` and `xbe_mask`, will be read from.

Let's look at these some `define`s to get an idea of the sizes:

#define MAXPATHLEN 1024
#define	IBE_ARG_LEN_MAX	256
#define	IBE_INTERP_LEN_MAX	(MAXPATHLEN + IBE_ARG_LEN_MAX)
#define	IBE_MAGIC_MAX	256

`xbe_interpreter` is 1280 bytes.
`xbe_magic` is 256 bytes.
`xbe_mask` is 256 bytes.

Now that we know we control the 512 bytes after the designated area for `xbe_interpreter`, let's go back to the target code:

	char t[IBE_INTERP_LEN_MAX];
	...
	sp = str; tp = t;
	while (*sp != '\0') {
		if (*sp == ' ' || *sp == '\t') {
			if (++len > IBE_INTERP_LEN_MAX)
				break;
			*tp++ = ' ';
			argc++;
			while (*sp == ' ' || *sp == '\t')
				sp++;
			continue;
		} else {
			*tp++ = *sp++;
			len++;
		}
	}
	*tp = '\0';

We fill our `xbe_interpreter` (`str`) with non-whitespace, ASCII bytes, 'a' for example so that the following code happens 1280 times:

    *tp++ = *sp++;

We then have stack overflow past `t` from user controlled contents `xbe_magic` and `xbe_mask` (ASCII values only) for any desired size up to the next 512 bytes (just place a 0 byte to end the overflow); we can use this to overflow return addresses on the stack to get kernel to jump anywhere we want as soon as this function returns, but only addresses with all ASCII bytes (but this isn't a problem).

Unfortunately, the sysctl node, `kern.binmisc.add` is only accessible as root.