OpenBSD mallocarray

Thu Feb 11 21:36:55 UTC 2016

> On Feb 11, 2016, at 2:21 PM, C Turt <cturt at hardenedbsd.org> wrote:
> 
> I just wanted to post some real world examples of bugs which could be
> mitigated with `mallocarray` to attract more interest.

Let’s play devil’s advocate: since you have to make code changes, how
would mallocarray() be superior to the various MALLOC_OVERFLOW
macro suggestions from earlier in the thread? Given that kernel code is
somewhat different in what it needs to support?

> 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.

Except that you’d need to change all the code that was imported into
the kernel to use mallocarray. The code Sony imported didn’t have it
to start with.

> 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`.

These are all good finds. And they are all mitigable with the MALLOC_OVERFLOW
macro that was suggested earlier in this thread. Given the unique demands of the
kernel, why should that not be the preferred method of dealing with this stuff?

Warner

> On Mon, Feb 1, 2016 at 7:57 PM, C Turt <cturt at 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 at 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));
>> }
>> 
>> 
> _______________________________________________
> freebsd-arch at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe at freebsd.org"

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20160211/d54a73ca/attachment.sig>