Skip to content

uc_mem_read/write/unmap/protect skip the address + size overflow check that uc_mem_map already has #2321

Description

@MarkLee131

uc_mem_map rejects address + size - 1 < address via mem_map_check (uc.c:1362). The other four memory APIs do not. They go straight through check_mem_area (uc.c:762), which iterates pages without an overflow check and walks from the top of the address space back to 0 if a low page is also mapped. Same source contract violation, different code path.

Severity is "API consistency / correctness" rather than security in the classical sense. The library never validates the buffer pointer, so a caller is already trusted to size the buffer correctly. But the existing check in uc_mem_map is the project's own statement that this is the right contract; the asymmetry is a bug.

Closes-or-revisits: #1641 (Segmentation fault in uc_mem_unmap with Unicorn 2, open since 2022).

Affected

  • uc_mem_read uc.c:913 (loop at 926-940)
  • uc_mem_write uc.c:951 (loop at 966-985)
  • uc_mem_unmap uc.c:1836
  • uc_mem_protect uc.c:1713

Repro

/* poc1_mem_read_wrap.c */
#include <stdio.h>
#include <stdlib.h>
#include <unicorn/unicorn.h>

int main(void) {
    uc_engine *uc;
    uc_open(UC_ARCH_X86, UC_MODE_64, &uc);
    uc_mem_map(uc, 0,                     0x1000, UC_PROT_ALL);
    uc_mem_map(uc, 0xFFFFFFFFFFFFF000ULL, 0x1000, UC_PROT_ALL);

    uint8_t *buf = malloc(0x1000);   /* intentionally too small */
    uc_mem_read(uc, 0xFFFFFFFFFFFFF000ULL, buf, 0x2000);
    /* returns UC_ERR_OK after writing 0x2000 bytes into a 0x1000 buffer */
}

Build with ASan and run. I tested two configurations and both reproduce:

# Configuration A: stock CMake debug build with ASan
mkdir build-asan && cd build-asan
CC=clang CFLAGS="-fsanitize=address,undefined -g -O1" \
  cmake .. -DUNICORN_ARCH=x86 -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=Debug
cmake --build . -j

# Compile and run the PoC
clang -fsanitize=address,undefined -g -O1 -I../include \
  poc1_mem_read_wrap.c libunicorn.a -lpthread -o poc1
ASAN_OPTIONS=detect_leaks=0 ./poc1

ASan output:

==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x...
WRITE of size 4096 at 0x... thread T0
    #0 __asan_memcpy
    #1 flatview_read_continue_x86_64 exec.c:1659
    #2 cpu_physical_memory_rw_x86_64 exec.c:1737
    #3 uc_mem_read uc.c:930
SUMMARY: AddressSanitizer: heap-buffer-overflow
uc_mem_read returned: OK (UC_ERR_OK)

The same setup against uc_mem_write mirrors this as a 4 KiB OOB read of the source buffer (poc5). For uc_mem_unmap and uc_mem_protect the call returns UC_ERR_OK and silently affects both regions; no immediate crash, but the internal mapped_blocks accounting is then inconsistent with what the caller asked for.

Suggested fix

Add the same guard mem_map_check already uses, hoisted into a tiny helper:

static bool addr_size_overflows(uint64_t a, uint64_t s) {
    return s != 0 && (a + s - 1) < a;
}

Call it at the top of all four APIs and return UC_ERR_ARG on overflow.
Happy to send a PR against dev with the helper plus a regression test in tests/unit/test_mem.c that closes #1641.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions