Harden the GGUF and GGML loaders against crafted files#3712
Open
v-code01 wants to merge 3 commits into
Open
Conversation
`Content::read` takes `general.alignment` straight from the file's
metadata and only rejects negative signed values; U8/U16/U32 (and
non-negative I8/I16/I32) pass through unchecked. A value of 0 then
reaches
position.div_ceil(alignment) * alignment
and panics with "attempt to divide by zero", so a crafted GGUF header
of a few bytes reliably crashes the loader (DoS on untrusted input).
The GGUF spec requires `general.alignment` to be a non-zero power of
two. Validate that and bail with a clear error before computing the
tensor-data offset. Adds a regression test that a header with
`general.alignment = 0` is rejected rather than panicking.
The legacy GGML reader trusts several length/size fields straight from the file, unlike the GGUF reader which was hardened in huggingface#3533: - `read_one_tensor` computes `dims.product() * type_size / block_size` with unchecked arithmetic, so a crafted shape (e.g. four dims of u32::MAX) panics with "attempt to multiply with overflow" in debug builds and wraps to a bogus small size in release builds. - The tensor `size_in_bytes`, `name_len`, per-token `len` and `n_vocab` are used directly as allocation lengths with no check against the remaining file size, so a tiny crafted header can request multi-GiB up-front allocations (allocation-abort DoS). - `n_dims` is an unbounded `u32` used as a `Vec` length. Mirror the GGUF hardening: cap `n_dims` at `GGML_MAX_DIMS` (4), fold the shape with `checked_mul`, and validate every file-derived length against the bytes remaining in the file before allocating. Malformed files now return a clear error instead of panicking or over-allocating. Adds `candle-core/tests/ggml_tests.rs` covering the shape overflow, oversized tensor data, and oversized tensor name cases.
`TensorInfo::read` computes `self.shape.elem_count()` (an unchecked `dims.iter().product()`) and then `tensor_elems / block_size * type_size()`, both with unchecked arithmetic. Tensor dimensions come straight from the file (up to 4 dims, each a full u64), so a crafted GGUF whose shape overflows `usize` — e.g. two dims of 2^40 — panics with "attempt to multiply with overflow" in debug builds when the tensor is loaded, and in release wraps to a bogus small `size_in_bytes` that passes the remaining-bytes check and yields a QTensor whose huge declared shape disagrees with its tiny storage (later out-of-bounds in matmul). Fold the shape with `checked_mul` and guard the `type_size` multiply, returning a clear error on overflow. This mirrors the GGML loader hardening in huggingface#3713; the GGUF path (`TensorInfo::read`) had been missed. Adds `rejects_tensor_shape_product_overflow` to gguf_tests.rs.
v-code01
force-pushed
the
gguf-reject-zero-alignment
branch
from
f1defd5 to
f923ce4
Compare
v-code01
changed the title
This was referenced Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The quantized model loaders take length/size/alignment fields straight from the file. Several are used unchecked, so a crafted
.gguf/.ggmlpanics or over-allocates on load. This PR fixes three (each with a regression test ingguf_tests/ggml_tests):general.alignment = 0position.div_ceil(0)→ divide-by-zero panicusize(e.g. two dims of 2^40)elem_count()/*type_sizeoverflow → panic (debug) / wrap-then-OOB (release)checked_mulfold → clean errorn_vocab/name_len/per-tokenlenused as alloc sizeschecked_mul+ validate every length against remaining file bytes;n_dimscappedThe GGUF metadata reader was already hardened in #3533; this brings the tensor-load path and the GGML reader up to the same bar.
Why one PR
All three are the same class (untrusted quantized-file field → panic/over-alloc) and use the same checked-arithmetic + remaining-bytes-validation approach; one coherent hardening pass is easier to review than three fragments.
Tests
rejects_zero_general_alignment,rejects_tensor_shape_product_overflow(gguf_tests), and a newggml_tests.rs(shape overflow, oversized tensor data, oversized name). Existinggguf_tests/quantized_testsstay green;cargo fmt --all+clippy -D warningsclean.cc @LaurentMazare @ivarflakstad @EricLBuehler