Skip to content

Commit e5bc588

Browse files
committed
gguf : reject empty metadata key names during parsing
Fixes #20873: a malformed GGUF file with a zero-length key string triggers GGML_ASSERT(!key.empty()) in the gguf_kv constructor. Add an early check in gguf_init_from_file_ptr that rejects empty keys with an error log, consistent with existing duplicate-key and bad-key-size validation. Three new handcrafted test cases: - KV_EMPTY_KEY: all keys are empty - KV_EMPTY_KEY_ONLY_FIRST: only the first key is empty - KV_EMPTY_KEY_ONLY_MID: a middle key is empty
1 parent f535774 commit e5bc588

2 files changed

Lines changed: 25 additions & 2 deletions

File tree

ggml/src/gguf.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,10 @@ struct gguf_context * gguf_init_from_file_ptr(FILE * file, struct gguf_init_para
508508
GGML_LOG_ERROR("%s: encountered bad_alloc error while reading key %" PRIi64 "\n", __func__, i);
509509
ok = false;
510510
}
511+
if (ok && key.empty()) {
512+
GGML_LOG_ERROR("%s: empty key name at KV pair %" PRIi64 "\n", __func__, i);
513+
ok = false;
514+
}
511515
for (size_t j = 0; ok && j < ctx->kv.size(); ++j) {
512516
if (key == ctx->kv[j].key) {
513517
GGML_LOG_ERROR("%s: duplicate key '%s' for tensors %zu and %" PRIi64 " \n", __func__, key.c_str(), j, i);

tests/test-gguf.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ enum handcrafted_file_type {
2929
HANDCRAFTED_KV_BAD_TYPE = 20 + offset_has_kv,
3030
// HANDCRAFTED_KV_BAD_VALUE_SIZE = 30 + offset_has_kv, // removed because it can result in allocations > 1 TB (default sanitizer limit)
3131
HANDCRAFTED_KV_DUPLICATE_KEY = 40 + offset_has_kv,
32+
HANDCRAFTED_KV_EMPTY_KEY = 45 + offset_has_kv,
33+
HANDCRAFTED_KV_EMPTY_KEY_ONLY_FIRST = 46 + offset_has_kv,
34+
HANDCRAFTED_KV_EMPTY_KEY_ONLY_MID = 47 + offset_has_kv,
3235
HANDCRAFTED_KV_BAD_ALIGN = 50 + offset_has_kv,
3336
HANDCRAFTED_KV_SUCCESS = 800 + offset_has_kv,
3437

@@ -66,6 +69,9 @@ static std::string handcrafted_file_type_name(const enum handcrafted_file_type h
6669
case HANDCRAFTED_KV_BAD_KEY_SIZE: return "KV_BAD_KEY_SIZE";
6770
case HANDCRAFTED_KV_BAD_TYPE: return "KV_BAD_TYPE";
6871
case HANDCRAFTED_KV_DUPLICATE_KEY: return "KV_DUPLICATE_KEY";
72+
case HANDCRAFTED_KV_EMPTY_KEY: return "KV_EMPTY_KEY";
73+
case HANDCRAFTED_KV_EMPTY_KEY_ONLY_FIRST: return "KV_EMPTY_KEY_ONLY_FIRST";
74+
case HANDCRAFTED_KV_EMPTY_KEY_ONLY_MID: return "KV_EMPTY_KEY_ONLY_MID";
6975
case HANDCRAFTED_KV_BAD_ALIGN: return "KV_BAD_ALIGN";
7076
case HANDCRAFTED_KV_SUCCESS: return "KV_RANDOM_KV";
7177

@@ -250,14 +256,24 @@ static FILE * get_handcrafted_file(const unsigned int seed, const enum handcraft
250256

251257
const std::string key = "my_key_" + std::to_string((hft == HANDCRAFTED_KV_DUPLICATE_KEY ? i/2 : i));
252258

253-
if (hft == HANDCRAFTED_KV_BAD_KEY_SIZE) {
259+
const bool empty_key_this_iter =
260+
hft == HANDCRAFTED_KV_EMPTY_KEY ||
261+
(hft == HANDCRAFTED_KV_EMPTY_KEY_ONLY_FIRST && i == 0) ||
262+
(hft == HANDCRAFTED_KV_EMPTY_KEY_ONLY_MID && i == int(kv_types.size()) / 2);
263+
264+
if (empty_key_this_iter) {
265+
const uint64_t n = 0;
266+
helper_write(file, n);
267+
} else if (hft == HANDCRAFTED_KV_BAD_KEY_SIZE) {
254268
const uint64_t n = -1;
255269
helper_write(file, n);
256270
} else {
257271
const uint64_t n = key.length();
258272
helper_write(file, n);
259273
}
260-
helper_write(file, key.data(), key.length());
274+
if (!empty_key_this_iter) {
275+
helper_write(file, key.data(), key.length());
276+
}
261277

262278
{
263279
const int32_t type32 = int32_t(type);
@@ -698,6 +714,9 @@ static std::pair<int, int> test_handcrafted_file(const unsigned int seed) {
698714
HANDCRAFTED_KV_BAD_KEY_SIZE,
699715
HANDCRAFTED_KV_BAD_TYPE,
700716
HANDCRAFTED_KV_DUPLICATE_KEY,
717+
HANDCRAFTED_KV_EMPTY_KEY,
718+
HANDCRAFTED_KV_EMPTY_KEY_ONLY_FIRST,
719+
HANDCRAFTED_KV_EMPTY_KEY_ONLY_MID,
701720
HANDCRAFTED_KV_BAD_ALIGN,
702721
HANDCRAFTED_KV_SUCCESS,
703722

0 commit comments

Comments
 (0)