Bound file-declared length prefixes in the model loader (heap overflow)

[gigioneggiando]

Summary

Two sites in the model-file loader read a uint64 length straight from a downloaded file and then allocate + read with no bound against the file:

• shard_load() — the safetensors header length;
• read_gguf_string_fp() — a GGUF string length.

Both do xmalloc((size_t)len + 1) then fread(len). A crafted length near SIZE_MAX makes (size_t)len + 1 wrap to 0, so xmalloc() hands back a 1-byte buffer (xmalloc maps 0 to malloc(1)); the following fread() of the file's real bytes then overflows it — a heap-buffer-overflow write. A large-but-non-wrapping length instead requests a multi-GB allocation from a tiny file (DoS).

These parse files people download and convert (quantized models shared online), so the input is attacker-influenced.

Fix

Add read_checked_len_fp(), which reads the length prefix and rejects it if it exceeds the bytes actually remaining in the file, and use it at both sites. Valid lengths (<= file size, so no size_t wrap) are unchanged; the file position is restored after the size check. Uses ftello/fseeko, already used elsewhere in this file — no new includes.

Verification

Built the real shard_load / read_gguf_string_fp under ASan and drove crafted files with len = SIZE_MAX:

• before: AddressSanitizer: heap-buffer-overflow WRITE (shard_load alloc→fread, and the GGUF-string sibling).
• after: clean rejection — error: safetensors header length (…) exceeds 64 bytes remaining in file, no sanitizer error.

Found during a coordinated security review of ds4; standalone offline PoCs available on request.