Skip to content

gguf : harden loader against malformed tensor dims and metadata types #25596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggerganov merged 3 commits into from
Aug 12, 2026
Merged

gguf : harden loader against malformed tensor dims and metadata types #25596

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4705c84
gguf : harden loader against malformed tensor dims and metadata types
harrison001 Jul 12, 2026
3cd5206
gguf: address review on malformed-metadata hardening
harrison001 Jul 13, 2026
c970b26
cont : less comments
ggerganov Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions ggml/src/gguf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,13 @@ static struct gguf_context * gguf_init_from_reader(const struct gguf_reader & gr
GGML_ASSERT(int64_t(ctx->kv.size()) == n_kv);

const int alignment_idx = gguf_find_key(ctx, GGUF_KEY_GENERAL_ALIGNMENT);
if (alignment_idx != -1 && gguf_get_kv_type(ctx, alignment_idx) != GGUF_TYPE_UINT32) {
GGML_LOG_ERROR("%s: key '%s' must be of type %s but is %s\n",
__func__, GGUF_KEY_GENERAL_ALIGNMENT, gguf_type_name(GGUF_TYPE_UINT32),
gguf_type_name(gguf_get_kv_type(ctx, alignment_idx)));
gguf_free(ctx);
return nullptr;
}
ctx->alignment = alignment_idx == -1 ? GGUF_DEFAULT_ALIGNMENT : gguf_get_val_u32(ctx, alignment_idx);

if (ctx->alignment == 0 || (ctx->alignment & (ctx->alignment - 1)) != 0) {
Expand Down Expand Up @@ -682,9 +689,11 @@ static struct gguf_context * gguf_init_from_reader(const struct gguf_reader & gr
}

// check that the total number of elements is representable
if (ok && ((INT64_MAX/info.t.ne[1] <= info.t.ne[0]) ||
(INT64_MAX/info.t.ne[2] <= info.t.ne[0]*info.t.ne[1]) ||
(INT64_MAX/info.t.ne[3] <= info.t.ne[0]*info.t.ne[1]*info.t.ne[2]))) {
// (a zero-element tensor is trivially representable; the guard also avoids a division by zero below)
if (ok && ggml_nelements(&info.t) > 0 &&
((INT64_MAX/info.t.ne[1] <= info.t.ne[0]) ||
(INT64_MAX/info.t.ne[2] <= info.t.ne[0]*info.t.ne[1]) ||
(INT64_MAX/info.t.ne[3] <= info.t.ne[0]*info.t.ne[1]*info.t.ne[2]))) {

GGML_LOG_ERROR("%s: total number of elements in tensor '%s' with shape "
"(%" PRIi64 ", %" PRIi64 ", %" PRIi64 ", %" PRIi64 ") is >= %" PRIi64 "\n",
Expand Down
44 changes: 36 additions & 8 deletions tests/test-gguf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ enum handcrafted_file_type {
// HANDCRAFTED_KV_BAD_VALUE_SIZE = 30 + offset_has_kv, // removed because it can result in allocations > 1 TB (default sanitizer limit)
HANDCRAFTED_KV_DUPLICATE_KEY = 40 + offset_has_kv,
HANDCRAFTED_KV_BAD_ALIGN = 50 + offset_has_kv,
HANDCRAFTED_KV_WRONG_TYPE_ALIGN = 55 + offset_has_kv,
HANDCRAFTED_KV_SUCCESS = 800 + offset_has_kv,

HANDCRAFTED_TENSORS_BAD_NAME_SIZE = 10 + offset_has_tensors,
HANDCRAFTED_TENSORS_BAD_N_DIMS = 20 + offset_has_tensors,
HANDCRAFTED_TENSORS_BAD_SHAPE = 30 + offset_has_tensors,
HANDCRAFTED_TENSORS_ZERO_DIM = 35 + offset_has_tensors,
HANDCRAFTED_TENSORS_NE_TOO_BIG = 40 + offset_has_tensors,
HANDCRAFTED_TENSORS_NBYTES_TOO_BIG = 45 + offset_has_tensors,
HANDCRAFTED_TENSORS_BAD_TYPE = 50 + offset_has_tensors,
Expand Down Expand Up @@ -69,11 +71,13 @@ static std::string handcrafted_file_type_name(const enum handcrafted_file_type h
case HANDCRAFTED_KV_BAD_TYPE: return "KV_BAD_TYPE";
case HANDCRAFTED_KV_DUPLICATE_KEY: return "KV_DUPLICATE_KEY";
case HANDCRAFTED_KV_BAD_ALIGN: return "KV_BAD_ALIGN";
case HANDCRAFTED_KV_WRONG_TYPE_ALIGN: return "KV_WRONG_TYPE_ALIGN";
case HANDCRAFTED_KV_SUCCESS: return "KV_RANDOM_KV";

case HANDCRAFTED_TENSORS_BAD_NAME_SIZE: return "TENSORS_BAD_NAME_SIZE";
case HANDCRAFTED_TENSORS_BAD_N_DIMS: return "TENSORS_BAD_N_DIMS";
case HANDCRAFTED_TENSORS_BAD_SHAPE: return "TENSORS_BAD_SHAPE";
case HANDCRAFTED_TENSORS_ZERO_DIM: return "TENSORS_ZERO_DIM";
case HANDCRAFTED_TENSORS_NE_TOO_BIG: return "TENSORS_NE_TOO_BIG";
case HANDCRAFTED_TENSORS_NBYTES_TOO_BIG: return "TENSORS_NBYTES_TOO_BIG";
case HANDCRAFTED_TENSORS_BAD_TYPE: return "TENSORS_BAD_TYPE";
Expand All @@ -95,6 +99,11 @@ static std::string handcrafted_file_type_name(const enum handcrafted_file_type h
}

static bool expect_context_not_null(const enum handcrafted_file_type hft) {
// a tensor with a zero-size dimension (0 total elements) is malformed but valid to load:
// it must not crash and the loader is expected to accept it
if (hft == HANDCRAFTED_TENSORS_ZERO_DIM) {
return true;
}
Comment thread
ggerganov marked this conversation as resolved.
Outdated
if (hft < offset_has_kv) {
return hft >= HANDCRAFTED_HEADER_EMPTY;
}
Expand Down Expand Up @@ -257,9 +266,9 @@ static FILE * get_handcrafted_file(const unsigned int seed, const enum handcraft
}
{
uint64_t n_kv = kv_types.size();
if (hft == HANDCRAFTED_KV_BAD_ALIGN ||
hft == HANDCRAFTED_TENSORS_BAD_ALIGN || hft == HANDCRAFTED_TENSORS_CUSTOM_ALIGN ||
hft == HANDCRAFTED_DATA_BAD_ALIGN || hft == HANDCRAFTED_DATA_CUSTOM_ALIGN) {
if (hft == HANDCRAFTED_KV_BAD_ALIGN || hft == HANDCRAFTED_KV_WRONG_TYPE_ALIGN ||
hft == HANDCRAFTED_TENSORS_BAD_ALIGN || hft == HANDCRAFTED_TENSORS_CUSTOM_ALIGN ||
hft == HANDCRAFTED_DATA_BAD_ALIGN || hft == HANDCRAFTED_DATA_CUSTOM_ALIGN) {

n_kv += 1;
} else if (hft == HANDCRAFTED_HEADER_BAD_N_KV) {
Expand Down Expand Up @@ -344,15 +353,17 @@ static FILE * get_handcrafted_file(const unsigned int seed, const enum handcraft
helper_write(file, data, hft == HANDCRAFTED_KV_BAD_TYPE ? 1 : gguf_type_size(type));
}

if (hft == HANDCRAFTED_KV_BAD_ALIGN ||
hft == HANDCRAFTED_TENSORS_BAD_ALIGN || hft == HANDCRAFTED_TENSORS_CUSTOM_ALIGN ||
hft == HANDCRAFTED_DATA_BAD_ALIGN || hft == HANDCRAFTED_DATA_CUSTOM_ALIGN) {
if (hft == HANDCRAFTED_KV_BAD_ALIGN || hft == HANDCRAFTED_KV_WRONG_TYPE_ALIGN ||
hft == HANDCRAFTED_TENSORS_BAD_ALIGN || hft == HANDCRAFTED_TENSORS_CUSTOM_ALIGN ||
hft == HANDCRAFTED_DATA_BAD_ALIGN || hft == HANDCRAFTED_DATA_CUSTOM_ALIGN) {

const uint64_t n = strlen(GGUF_KEY_GENERAL_ALIGNMENT);
helper_write(file, n);
helper_write(file, GGUF_KEY_GENERAL_ALIGNMENT, n);

const int32_t type = gguf_type(GGUF_TYPE_UINT32);
// HANDCRAFTED_KV_WRONG_TYPE_ALIGN declares general.alignment with a non-UINT32 type,
// which the loader must reject cleanly instead of aborting on an assertion
const int32_t type = hft == HANDCRAFTED_KV_WRONG_TYPE_ALIGN ? int32_t(GGUF_TYPE_INT32) : int32_t(GGUF_TYPE_UINT32);
helper_write(file, type);

alignment = expect_context_not_null(hft) ? 1 : 13;
Expand Down Expand Up @@ -403,6 +414,9 @@ static FILE * get_handcrafted_file(const unsigned int seed, const enum handcraft
break;
}
}
if (hft == HANDCRAFTED_TENSORS_ZERO_DIM) {
n_dims = 2;
}
if (hft == HANDCRAFTED_TENSORS_BAD_N_DIMS) {
const uint32_t n_dims_bad = GGML_MAX_DIMS + 1;
helper_write(file, n_dims_bad);
Expand All @@ -415,6 +429,11 @@ static FILE * get_handcrafted_file(const unsigned int seed, const enum handcraft
for (uint32_t j = 0; j < n_dims; ++j) {
helper_write(file, bad_dim);
}
} else if (hft == HANDCRAFTED_TENSORS_ZERO_DIM) {
// shape with a zero-size trailing dimension: ne = {ne[0], 0}, i.e. 0 total elements.
// ne[0] stays a multiple of the type's block size; ne[1] == 0 is what used to divide by zero.
const int64_t zero_shape[2] = { shape[0], 0 };
helper_write(file, zero_shape, 2*sizeof(int64_t));
Comment thread
ggerganov marked this conversation as resolved.
} else if (hft == HANDCRAFTED_TENSORS_NE_TOO_BIG){
const int64_t big_dim = 4*int64_t(INT32_MAX);
for (uint32_t j = 0; j < n_dims; ++j) {
Expand Down Expand Up @@ -446,6 +465,11 @@ static FILE * get_handcrafted_file(const unsigned int seed, const enum handcraft
for (uint32_t i = 1; i < n_dims; ++i) {
ne *= shape[i];
}
if (hft == HANDCRAFTED_TENSORS_ZERO_DIM) {
// the crafted tensors have 0 elements, so each occupies 0 bytes: keep the
// written offsets consistent with what the loader computes (all zero)
ne = 0;
}
Comment thread
ggerganov marked this conversation as resolved.

offset += GGML_PAD(ggml_row_size(type, ne), (uint64_t) alignment);
}
Expand Down Expand Up @@ -740,11 +764,13 @@ static std::pair<int, int> test_handcrafted_file(const unsigned int seed) {
HANDCRAFTED_KV_BAD_TYPE,
HANDCRAFTED_KV_DUPLICATE_KEY,
HANDCRAFTED_KV_BAD_ALIGN,
HANDCRAFTED_KV_WRONG_TYPE_ALIGN,
HANDCRAFTED_KV_SUCCESS,

HANDCRAFTED_TENSORS_BAD_NAME_SIZE,
HANDCRAFTED_TENSORS_BAD_N_DIMS,
HANDCRAFTED_TENSORS_BAD_SHAPE,
HANDCRAFTED_TENSORS_ZERO_DIM,
HANDCRAFTED_TENSORS_NE_TOO_BIG,
HANDCRAFTED_TENSORS_NBYTES_TOO_BIG,
HANDCRAFTED_TENSORS_BAD_TYPE,
Expand Down Expand Up @@ -833,7 +859,9 @@ static std::pair<int, int> test_handcrafted_file(const unsigned int seed) {
ntest++;
}

if (expect_context_not_null(hft) && hft >= offset_has_tensors) {
// HANDCRAFTED_TENSORS_ZERO_DIM deliberately mangles the tensor shapes to 0 elements,
// so only assert that it loads without crashing; skip the exact-geometry comparison.
if (expect_context_not_null(hft) && hft >= offset_has_tensors && hft != HANDCRAFTED_TENSORS_ZERO_DIM) {
printf("%s: - check_tensors: ", __func__);
if (handcrafted_check_tensors(gguf_ctx, seed)) {
printf("\033[1;32mOK\033[0m\n");
Expand Down
Loading