DEV Community

Cover image for Your Model File Is Untrusted Input
Harrison Guo
Harrison Guo

Posted on Originally published at harrisonsec.com

Your Model File Is Untrusted Input

Sixty-five bytes is not a model. It is not even a header with something plausible after it. It is a GGUF magic number, a few counts, and a tensor whose second dimension is zero.

Load it and llama.cpp dies.

Not with a memory error, and not because anything was corrupted. It dies dividing by a number that the file format is supposed to allow.

The loader is the trust boundary

Nobody thinks of a model file as input. You download a .gguf from a hub the way you download a JPEG, and then a C++ binary parses it into heap allocations, tensor shapes and metadata keys. Everything downstream reaches the same parser: llama-server, the bindings, the desktop apps that wrap them.

gguf_init_from_buffer is where a byte array becomes a model. That makes it the boundary, and boundaries are worth fuzzing.

The harness is unremarkable: libFuzzer and AddressSanitizer pointed at that one function. Binary formats with length fields are where parsers go wrong, and a model file is a binary format that is nothing but length fields.

Sixty-five bytes, field by field

The file that kills it, laid out:

offset size field value
0 4 magic GGUF
4 4 version 3
8 8 tensor count 1
16 8 metadata kv count 0
24 8 name length 1
32 1 tensor name t
33 4 n_dims 2
37 8 ne[0] 1
45 8 ne[1] 0
53 4 type 0 (F32)
57 8 offset 0

A version the loader supports. One tensor, no metadata, a one-character name, two dimensions, the first of them 1. Fifty-seven of those sixty-five bytes are the least remarkable file you could construct.

The other eight are at offset 45, and what makes them fatal is that they are zero, not that they are wrong. Nothing here is malformed. There is no length lying about a buffer, no count that overruns an allocation, no string missing its terminator, none of the things a parser is written to be suspicious of. Every field is in range, including that one.

The first crash, and the check that was already there

Tensor dimensions are validated. The loader checks that each one is non-negative, which reads like exactly the right thing to do, and it has been there a long time.

Then the next check tests whether the element count can be represented at all:

if (ok && ((INT64_MAX/info.t.ne[1] <= info.t.ne[0]) || ...
Enter fullscreen mode Exit fullscreen mode

That divides by ne[1]. A dimension of zero passes the first check, because zero is not negative, and arrives at the second, where INT64_MAX / 0 raises SIGFPE and the process is gone.

AddressSanitizer: FPE ... gguf.cpp in gguf_init_from_reader(...)
Enter fullscreen mode Exit fullscreen mode

What makes this one worth writing down is not the arithmetic. It is that a validation check sat directly above the crash, and it passed review, and it is correct as far as it goes. ne[j] >= 0 is a true statement about what a dimension may be. It just is not the statement the line below it needed. A check that half-covers a value is worse than no check, because the reader below it stops asking. That is the wrong ruler again, a check that reads as complete and quietly is not.

Zero is legal, so rejecting it was never the fix

The obvious patch is to demand a positive dimension. That patch is wrong.

Zero-sized tensors are allowed on purpose. They exist in real files, where some diffusion models carry them as feature tags, marking a capability rather than holding data, and safetensors permits them too. A loader that began refusing them would reject models that are not malformed at all.

So the fix is not about the input. It is about the arithmetic: skip the representability check when a dimension is zero, because an element count of zero is trivially representable. Nothing is rejected that was not already rejected. The division simply stops happening on a value that was always going to be there.

This came up in review. The maintainer's recollection was that zero-sized tensors were tolerated but pointless; a collaborator corrected it with the diffusion case. The correction is the reason the patch stayed narrow.

The second crash: a type nobody checked

The other one is smaller and more ordinary.

GGUF metadata is typed. Each key declares what it holds, and the reader is supposed to honour that declaration. general.alignment is read as a uint32, without asking what the file said it was.

A file that declares general.alignment with any other type reaches a template whose whole job is to notice that mismatch:

GGML_ASSERT(type_to_gguf_type<T>::value == type) failed
Enter fullscreen mode Exit fullscreen mode

An assertion is not a safety mechanism against hostile input. It is a statement that something impossible has happened, and its response is to abort. Here it was reachable from a 62-byte file, which means the impossible thing was merely undeclared.

The fix is a type check before the read, and a clean failure instead of an abort.

What review changed

I opened the pull request with the fix and the two reproducers. Three things came back, and each one made it better than what I sent.

Tests. The first comment asked for regression tests, which was the right call and the part I had skipped. Two handcrafted files went into tests/test-gguf.cpp: a metadata key with the wrong type, and a tensor with a zero-size dimension. Both crash the old build and load cleanly against the fix. The suite went 164/164.

The fix itself. My patch tested the dimensions one at a time. The review asked for ggml_nelements(info.t) > 0 instead. Identical behaviour, but it says what it means: skip the overflow check when there are no elements to overflow. Checking each dimension is how you think while you are staring at a division. Checking the element count is what the condition was always about, and it is one statement instead of several, which matters for a check whose whole failure mode was that a reader trusted it without enumerating the cases.

The error message. Rejecting a wrongly-typed key is not much help if it won't say what it wanted. It now reports both sides: key 'general.alignment' must be of type u32 but is i32.

That exchange is worth more than the patch. A fix without a regression test lasts until the next refactor, and a fix phrased as three separate conditions is an invitation for someone to eventually need a fourth.

Someone else found it the same week

Nine days after the pull request went up, a comment appeared on it from someone running an unrelated campaign against gguf_init_from_buffer, libFuzzer with ASan and UBSan at around 400,000 iterations, who had landed on the same SIGFPE by the same route.

That part is only mildly interesting. Two fuzzers aimed at the same function find the same shallow bug; that is what shallow means.

What was worth reading is what they did next. They went through the surrounding overflow check by hand to see whether the patch was complete:

the ne[0]/ne[2]/ne[3] multiplications can't independently overflow due to the || short-circuit, so the zero-check in this PR looks like it fully covers the defect rather than being a partial fix

That is the right question, and it is the same question this bug was made of. ne[j] >= 0 was a partial guarantee that read as a complete one, and it went unexamined precisely because it was there. Somebody checking whether its replacement had the same shape is the review this class of defect needs and almost never gets.

The part that generalises

Two things, neither of them about GGUF.

A downloaded model deserves the suspicion you give a downloaded binary. It is parsed by native code before anything else happens to it. These two bugs stop at denial of service, which is survivable when you are crashing your own process and much less so when a service loads models on someone else's behalf. The category is the point, not the severity.

Validation that half-covers a value is a liability. ne[j] >= 0 is not wrong. It is simply not the guarantee the line below it depended on, and its presence is what made the line below it look safe. When you find a bounds check, the useful question is not whether it is correct but what the code underneath it now assumes. That is the same reflex as reading a signal that is present but not diagnostic: the check ran, and running is not the claim you needed it to make.


The fix is in llama.cpp#25596, merged August 2026, with both reproducers in the description. If you run models locally, you already have the patched loader.

This is a Parser Field Note. It shares its spine with A Wrong Ruler Is Worse Than No Ruler on a check that reads as complete and is not, and The Log Printed Exactly What I Wanted on a true signal that supports a smaller claim than you lean on it for.

Top comments (0)