Integer overflow in bounds_check

[joachimff]

The bounds_check function in ubpf_vm.c has two checks that are susceptible to buffer overflow if addr value is to high. This could lead to incorrect memory access and possibly cause a segmentation fault and a crash.

Here is an example of a payload that would trigger the overflow and try to write 0 to the 0xFFFFFFFFFFF address:

0xb7, 0x0, 0x6, 0x0, 0x0 //r[6] = 0
0x17, 0x0, 0x6, 0x0, 0x1 //[r6] = 0xFFFFFFFFFFF
0x7A, 0x0, 0x6, 0x0, 0x0 //*(*(uint64_t *) (r[6] + off) = imm => * (0xFFFFFFF) = 0

To address this issue, I propose adding a check to ensure that the value of addr + size does not cause an integer overflow before performing the bounds check. If the check fails, the function will print an error message and return false.

Here is the updated code for the bounds_check function:

bool bounds_check(const void *addr, size_t size, const void *mem, size_t mem_len, const void *stack, struct ubpf_vm *vm, const char *type, uint32_t cur_pc)
{
    if ((uint64_t)addr + size < (uint64_t)addr){
        vm->error_printf(
            stderr,
            "uBPF error: integer overflow %s at PC %u, addr %p, size %zd\nmem %p/%zd stack %p/%d\n",
            type,
            cur_pc,
            addr,
            size,
            mem,
            mem_len,
            stack,
            UBPF_STACK_SIZE);
        return false;
    }
    else if (mem && (addr >= mem && ((char*)addr + size) <= ((char*)mem + mem_len))) {
        /* Context access */
        return true;
    } else if (addr >= stack && ((char*)addr + size) <= ((char*)stack + UBPF_STACK_SIZE)) {
        /* Stack access */
        return true;
    } else {
        vm->error_printf(
            stderr,
            "uBPF error: %s out of bounds at PC %u, addr %p, size %zd\nmem %p/%zd stack %p/%d\n",
            type,
            cur_pc,
            addr,
            size,
            mem,
            mem_len,
            stack,
            UBPF_STACK_SIZE);
        return false;
    }
}

ubpf_vm.c:995

With this change, the bounds_check function will check for integer overflow before performing the bounds check, ensuring that the function is protected against buffer overflow.

[Alan-Jowett]

Thanks @joachimff. Would it be possible for you to create a PR with this change?