Date: Thu, 11 Feb 2016 21:21:42 +0000 From: C Turt <cturt@hardenedbsd.org> To: arch@freebsd.org Subject: Re: OpenBSD mallocarray Message-ID: <CAB815Zb9BZcMiHg%2BgkExyCaDbMqqJUNK_X-X2Z8sw54QZ1SRwA@mail.gmail.com> In-Reply-To: <CAB815ZafpqJoqr1oH8mDJM=0RxLptQJpoJLexw6P6zOi7oSXTQ@mail.gmail.com> References: <CAB815ZafpqJoqr1oH8mDJM=0RxLptQJpoJLexw6P6zOi7oSXTQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
I just wanted to post some real world examples of bugs which could be mitigated with `mallocarray` to attract more interest. My most meritable example of a real world attack from this behaviour would be the sys_dynlib_prepare_dlclose kernel exploit for PS4 (PS4 OS is based on FreeBSD 9.0). You may read my write-up of this exploit here: http://cturt.github.io/dlclose-overflow.html The significance of this example is that if a `mallocarray` wrapper was available, and used here, the bug would not have been exploitable, because it would have intentionally panicked instead of continuing with an under allocated buffer. You may think that this is clearly Sony's fault for not checking the count at all, and that FreeBSD kernel code would never have a bug like this, which is why I would like to mention that similar overflows can be possible even if there initially appear to be sufficient bound checks performed. There are several pieces of vulnerable code present in FreeBSD kernel (albeit most of them are triggerable only as root so are not critical), however I will use the file `/sys/kern/kern_alq.c` to demonstrate. There are some checks on counts and sizes, but they are insufficient: [CODE]int alq_open(struct alq **alqp, const char *file, struct ucred *cred, int cmode, int size, int count) { int ret; KASSERT((count >= 0), ("%s: count < 0", __func__)); if (count > 0) { if ((ret = alq_open_flags(alqp, file, cred, cmode, size*count, 0)) == 0) { (*alqp)->aq_flags |= AQ_LEGACY; (*alqp)->aq_entmax = count; (*alqp)->aq_entlen = size; } ... int alq_open_flags(struct alq **alqp, const char *file, struct ucred *cred, int cmode, int size, int flags) { ... KASSERT((size > 0), ("%s: size <= 0", __func__)); ... alq->aq_buflen = size; ... alq->aq_entbuf = malloc(alq->aq_buflen, M_ALD, M_WAITOK|M_ZERO);[/CODE] The check on `count` being greater than or equal to 0 in `alq_open`, and the check for `size` being greater than 0 in `alq_open_flags` are cute, but they don't check for an overflow of `size*count`. This code path is reachable in several places, such as `sysctl_debug_ktr_alq_enable`: [CODE]static int sysctl_debug_ktr_alq_enable(SYSCTL_HANDLER_ARGS) { ... error = alq_open(&ktr_alq, (const char *)ktr_alq_file, req->td->td_ucred, ALQ_DEFAULT_CMODE, sizeof(struct ktr_entry), ktr_alq_depth); [/CODE] With `ktr_alq_depth` being controllable from userland (but only as root): [CODE]SYSCTL_INT(_debug_ktr, OID_AUTO, alq_depth, CTLFLAG_RW, &ktr_alq_depth, 0, "Number of items in the write buffer");[/CODE] `sizeof(struct ktr_entry)` is 88 bytes. So theoretically if `ktr_alq_depth` is set to `48806447`, we'll get an allocation of `0x100000028`, which truncates to 0x28 = 40 bytes. A heap overflow would then possible when this buffer is iterated over with `aq_entmax` and `aq_entlen`. On Mon, Feb 1, 2016 at 7:57 PM, C Turt <cturt@hardenedbsd.org> wrote: > I've recently started browsing the OpenBSD kernel source code, and have > found the mallocarray function positively wonderful. I would like to > discuss the possibility of getting this into FreeBSD kernel. > > For example, many parts of kernel code in FreeBSD use something like > malloc(xxx * sizeof(struct xxx)). If xxx is 64bit and controllable by user, > this allocation can easily overflow, resulting in a heap overflow later on. > > The mallocarray is a wrapper for malloc which can be used in this > situations to detect an integer overflow before allocating: > > /* > * Copyright (c) 2008 Otto Moerbeek <otto@drijf.net> > * > * Permission to use, copy, modify, and distribute this software for any > * purpose with or without fee is hereby granted, provided that the above > * copyright notice and this permission notice appear in all copies. > * > * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > */ > > /* > * This is sqrt(SIZE_MAX+1), as s1*s2 <= SIZE_MAX > * if both s1 < MUL_NO_OVERFLOW and s2 < MUL_NO_OVERFLOW > */ > #define MUL_NO_OVERFLOW (1UL << (sizeof(size_t) * 4)) > > void * > mallocarray(size_t nmemb, size_t size, int type, int flags) > { > if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) && > nmemb > 0 && SIZE_MAX / nmemb < size) { > if (flags & M_CANFAIL) > return (NULL); > panic("mallocarray: overflow %zu * %zu", nmemb, size); > } > return (malloc(size * nmemb, type, flags)); > } > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAB815Zb9BZcMiHg%2BgkExyCaDbMqqJUNK_X-X2Z8sw54QZ1SRwA>