Bug 207629 - Integer overflow in sysctl_kern_proc_args
Summary: Integer overflow in sysctl_kern_proc_args
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-01 22:25 UTC by CTurt
Modified: 2023-05-23 20:26 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description CTurt 2016-03-01 22:25:22 UTC
The `sysctl_kern_proc_args` (`CTLFLAG_ANYBODY`) handler (`sys/kern/kern_proc.c`) has an integer overflow vulnerability.

Relevant code:

static int
sysctl_kern_proc_args(SYSCTL_HANDLER_ARGS)
{
    int *name = (int *)arg1;
    u_int namelen = arg2;
    struct pargs *newpa, *pa;
    struct proc *p;
    struct sbuf sb;
    int flags, error = 0, error2;
   
    ...
   
    if (req->newlen + sizeof(struct pargs) > ps_arg_cache_limit)
        return (ENOMEM);
    newpa = pargs_alloc(req->newlen);
    error = SYSCTL_IN(req, newpa->ar_args, req->newlen);
   
    ...
}

struct pargs *
pargs_alloc(int len)
{
    struct pargs *pa;

    pa = malloc(sizeof(struct pargs) + len, M_PARGS,
        M_WAITOK);
    refcount_init(&pa->ar_ref, 1);
    pa->ar_length = len;
    return (pa);
}

If `newlen` is high enough such that `req->newlen + sizeof(struct pargs)` overflows, the check against `ps_arg_cache_limit` will be bypassed:

    if (req->newlen + sizeof(struct pargs) > ps_arg_cache_limit)
        return (ENOMEM);

For example, since `sizeof(struct pargs)` is `12`, passing `0xffffffffffffffff` will result in checking `11` against `ps_arg_cache_limit`.

Since `pargs_alloc` takes `len` as a `signed int`, the value passed to it will be `-1`, and the value passed to `malloc` will be `11`.

In `pargs_alloc` this pointer is then written to:

    refcount_init(&pa->ar_ref, 1);
    pa->ar_length = len;

And the code will then pass the full size of `0xffffffffffffffff` bytes to SYSCTL_IN of this allocation of `11` bytes:

    SYSCTL_IN(req, newpa->ar_args, req->newlen);

However, `SYSCTL_IN` has more checks than `copyin`, which makes it impossible for a buffer overflow to occur here.

There is another possibility though; if `req->newlen` is `-12`, the allocation will be 0, and the 2 writes in `pargs_alloc` will be out of bounds.

Disassembly of this (FreeBSD 9.0):

.text:FFFFFFFF8082646E                 call    malloc
.text:FFFFFFFF80826473                 mov     dword ptr [rax], 1
.text:FFFFFFFF80826479                 mov     [rax+4], ebx
Comment 1 CTurt 2016-03-01 22:26:42 UTC
PoC code which demonstrates the check being bypassed from the return values:

https://gist.github.com/CTurt/89c0544cb4dcc1fb8ce4
Comment 2 Mark Johnston freebsd_committer freebsd_triage 2023-05-23 20:26:15 UTC
Sorry that this didn't get attention when it was submitted.  I think this has since been fixed by commit 712dda7fb0b83, though it's possible that something else mitigated it before that.

> There is another possibility though; if `req->newlen` is `-12`, the allocation will be 0, and the 2 writes in `pargs_alloc` will be out of bounds.

An allocation length of zero will return a chunk of 16 bytes, so I don't think this could have resulted in an out-of-bounds write.