accessvector

Richard StaLLMan?

Thanks for your work in making an excellent tool; I, and many of my coworkers, use this. The price point is entirely fair and it’s a pleasure to use every time. But - above this - thank you also for prioritising personal life above development.

It’s great as it is and we’ll be happy to see new features when you’re ready. I would be really proud if I were in your position.

Cheers!

I started programming with QBASIC at about 10 years old, too, after randomly coming across a book called “Practise Your BASIC”. It’s a great book and Usborne recently released it for free: https://usborne.com/gb/books/computer-and-coding-books

Right, so this is the crux of the vulnerability. Firstly, note that the `MAX` macro is defined as:

    #define MAX(a, b) ((a) > (b) ? (a) : (b))

This is important because it doesn't cast either arg to any particular type. You can use it with `float`s, or `int`s, or `u_int`s... or a combination.

Referring back to the implementation, the first use of `MAX` (inside the `for` loop) is this:

    ...
    for (i = 0; i < nsyscalls; i++)
        npins = MAX(npins, syscalls[i].sysno);
    ...

`npins` is an `int`, but `sysno` is a `u_int`. C integer promotion rules means that we'll actually be implicitly casting `npins` to a `u_int` here; it's as if we did this:

    ...
     npins = MAX((u_int)npins, syscalls[i].sysno);
    ...

This means that `npins` can end up as any value we like -- even up to `0xffffffff`. But remember that `npins` is _actually_ a signed int, so once it comes out of `MAX`, it'll be signed again. Thus we can use this to make `npins` negative.

Once we're out of the loop, `MAX` is used again here:

    ...
    npins = MAX(npins, SYS_kbind);
    ...

Where `SYS_kbind` is just:

    ...
    #define SYS_kbind       86
    ...

Integer literals in C are signed, so now this use of `MAX` is actually dealing with two signed integers. If we used the loop to make `npins` negative (as described just before) then this line will now take 86 as the maximum of the two values.

With `npins = 86`, an array of 86+1 will be allocated, but the `syscalls[i].sysno` in the next loop could of course easily be greater than 86 -- thus leading to out-of-bounds array access.

Consider this:

    struct pinsyscall entries[] = {
        { .sysno = 1, .offset = 0x1234 },
        { .sysno = 2, .offset = 0x5678 },
        { .sysno = 1, .offset = 0x9abc }
    };

Now `nsyscalls` will be 3 and `pin` will be an array of 3 ints, initialised to `{ 0, 0, 0 }`.

When we loop through, we'll set:

    1. `pin[syscalls[0].sysno] = 0x1234` => `pin[1] = 0x1234`
    2. `pin[syscalls[1].sysno] = 0x5678` => `pin[2] = 0x5678`

Now when we come to 3, we'll find `pin[syscalls[2].sysno] != 0` since `syscalls[2].sysno == syscalls[0].sysno` - so we set `pin[1] = -1` instead of `0x9abc`.

Just to handle the case where the same syscall number is specified twice by the ELF header: in that case, the entry is set to -1 (presumably meaning it’s invalid).

Re-reading this, my analysis is slightly incorrect: the `MAX` at [5] with an unsigned arg means we can make `npins` an arbitrary `int` using the loop at [4].

Choosing to make `npins` negative using that loop means we'll end up allocating an array of 87 (`SYS_kbind + 1`) `int`s at [8] and continue with the OOB accesses described.

You'd set up your `pinsyscall` entries like this:

    struct pinsyscall entries[] = {
        { .sysno = 0x1111, .offset = 0xdeadbeef }, /* first oob write */
        { .sysno = 0x2222, .offset = 0xf000f000 }, /* second oob write */
        { .sysno = 0xffffffff } /* sets npins to 0xffffffff so we under-allocate */
    };

`npins` would be `0xffffffff` after the loop and then the `MAX` at [6] would then return `86`, since `MAX(-1, 86) == 86`.

Out-of-bounds heap write happens in this function:

        int
        elf_read_pintable(struct proc *p, Elf_Phdr *pp, struct vnode *vp,
            Elf_Ehdr *eh, uint **pinp)
        {
         struct pinsyscalls {
          u_int offset;
          u_int sysno;
         } *syscalls = NULL;
         int i, npins = 0, nsyscalls;
         uint *pins = NULL;
        
    [1]  nsyscalls = pp->p_filesz / sizeof(*syscalls);
         if (pp->p_filesz != nsyscalls * sizeof(*syscalls))
          goto bad;
    [2]  syscalls = malloc(pp->p_filesz, M_PINSYSCALL, M_WAITOK);
    [3]  if (elf_read_from(p, vp, pp->p_offset, syscalls,
             pp->p_filesz) != 0) {
          goto bad;
         }
        
    [4]  for (i = 0; i < nsyscalls; i++)
    [5]   npins = MAX(npins, syscalls[i].sysno);
    [6]  npins = MAX(npins, SYS_kbind);  /* XXX see ld.so/loader.c */
    [7]  npins++;
        
    [8]  pins = mallocarray(npins, sizeof(int), M_PINSYSCALL, M_WAITOK|M_ZERO);
         for (i = 0; i < nsyscalls; i++) {
    [9]   if (pins[syscalls[i].sysno])
    [10]   pins[syscalls[i].sysno] = -1; /* duplicated */
          else
    [11]   pins[syscalls[i].sysno] = syscalls[i].offset;
         }
         pins[SYS_kbind] = -1;   /* XXX see ld.so/loader.c */
        
         *pinp = pins;
         pins = NULL;
        bad:
         free(syscalls, M_PINSYSCALL, nsyscalls * sizeof(*syscalls));
         free(pins, M_PINSYSCALL, npins * sizeof(uint));
         return npins;
        }

So first of all we calculate the number of syscalls in the pin section [1], allocate some memory for it [2] and read it in [3].

At [4], we want to figure out how big to make our pin array, so we loop over all of the syscall entries and record the largest we've seen so far [5]. (Note: the use of `MAX` here is fine since `sysno` is unsigned -- see near the top of the function).

With the maximum `sysno` found, we then crucially go on to clamp the value to `SYS_kbind` [6] and +1 at [7].

This clamped maximum value is used for the array allocation at [8].

We now loop through the syscall list again, but now take the unclamped `sysno` as the index into the array to read at [9] and write at [10] and [11]. This is essentially the vulnerability right here.

Through heap grooming, there's a good chance you could arrange for a useful structure to be placed within range of the write at [11] -- and `offset` is essentially an arbitrary value you can write. So it looks like it would be relatively easy to exploit.

Spoiler for anyone else that got excited: nothing new has been published.

neovim became unusable for me when they removed cscope support, which I’ve depended on for my work for almost 15 years now.

That prompted me to return to regular vim and I’ve been perfectly happy since.

A pretty boring (non-exploitable) yet widespread use-after-free vulnerability that was recently patched and affected Linux kernels since ~2013. It involves a race condition between the exit path for a process and /proc/<pid>/timers.

In this post, I explain the race and walk through exploitability analysis.