MBM7's picture
Upload 2 files
27bfb5c verified
|
Raw
History Blame Contribute Delete
5.02 kB

GGUF Nested Array (ARRAY-of-ARRAY) Uncontrolled Recursion β€” RecursionError DoS

Status: Preparing for Huntr submission Package: gguf (PyPI) β€” official gguf-py from ggml-org/llama.cpp File / function: gguf/gguf_reader.py, GGUFReader._get_field_parts() Class: CWE-674 (Uncontrolled Recursion) Severity: Medium β€” an uncaught, unhandled exception crashes any application calling GGUFReader() on a ~12KB crafted file, with no exception type most code would think to catch.

Summary

When a GGUF key/value metadata field has type ARRAY, GGUFReader._get_field_parts() reads the array's element type and recursively calls itself to parse each element:

if gtype == GGUFValueType.ARRAY:
    raw_itype = self._get(offs, np.uint32)
    ...
    for idx in range(alen[0]):
        curr_size, curr_parts, curr_idxs, curr_types = self._get_field_parts(offs, raw_itype[0])

There is no check that the element type (raw_itype) isn't itself ARRAY, and no recursion depth limit. A GGUF file can declare an array whose elements are arrays, whose elements are arrays, nested as deep as the file's author chooses.

The reference C++ implementation explicitly rejects this. In ggml/src/gguf.cpp's type-dispatch switch:

case GGUF_TYPE_ARRAY:
default:
    {
        GGML_LOG_ERROR("%s: key '%s' has invalid GGUF type %d\n", ...);
        ok = false;
    } break;

Encountering ARRAY as an array's element type is treated as invalid input and cleanly rejected. The Python bindings have no equivalent check.

Proof of Concept

poc_gguf_nested_array_recursion.py builds a series of small, otherwise-valid GGUF files with a single KV field nested to increasing depth, and shows the divergence:

pip install gguf numpy
python poc_gguf_nested_array_recursion.py [path to a compiled llama-gguf binary, optional]

Results

Nesting depth File size Python gguf-py Native C++ reader
1 70 bytes accepted silently rejected cleanly ("invalid GGUF type")
10 178 bytes accepted silently (not tested, same as depth 1)
100 1,258 bytes accepted silently β€”
1,000 12,058 bytes RecursionError: maximum recursion depth exceeded β€”
5,000 60,058 bytes RecursionError: maximum recursion depth exceeded β€”

Even a single level of nesting is a genuine parity gap (Python accepts what the reference C++ implementation explicitly rejects as malformed), and by depth ~1000 β€” matching Python's default sys.setrecursionlimit() β€” it becomes an uncaught crash.

The RecursionError is not caught anywhere in gguf-py; it propagates all the way out of GGUFReader.__init__():

Traceback (most recent call last):
  File "...", line 3, in <module>
  File ".../gguf/gguf_reader.py", line 169, in __init__
    offs = self._build_fields(offs, kv_count)
  File ".../gguf/gguf_reader.py", line 298, in _build_fields
    field_size, field_parts, field_idxs, field_types = self._get_field_parts(offs, raw_kv_type[0])
  File ".../gguf/gguf_reader.py", line 248, in _get_field_parts
    curr_size, curr_parts, curr_idxs, curr_types = self._get_field_parts(offs, raw_itype[0])
  [Previous line repeated 990 more times]
  ...
RecursionError: maximum recursion depth exceeded

Any application calling GGUFReader(path) without a specific except RecursionError handler β€” unusual, since most file-parsing code anticipates ValueError/OSError, not RecursionError β€” crashes with an unhandled exception.

Impact

Any service that loads untrusted or third-party GGUF files with gguf-py (model preview/validation services, conversion tooling, CI harnesses testing community-submitted models) can be crashed by a ~12KB file that otherwise looks like an ordinary, if oddly-structured, metadata field.

Suggested fix

In _get_field_parts(), reject ARRAY as a valid array-element type the same way the C++ implementation does (this alone fixes the underlying parity gap and, as a side effect, removes the unbounded-recursion path entirely, since the recursion can only occur through nested arrays).

Relationship to other reports

Same vulnerability class (CWE-674, Uncontrolled Recursion) as a separately-reported finding in protobuf's json_format.ParseDict() β€” but a completely different library, format, and code path. Also distinct from this reporter's two other gguf-py findings: "KV Array Field Unbounded Length" (CWE-834, iteration-based, not recursion) and "Tensor Data Offset Aliasing" (CWE-1284, a data-integrity issue unrelated to metadata parsing). Also distinct from a separately and independently reported n_dims/GGML_MAX_DIMS parity gap (a different structural issue β€” per-tensor dimension count vs. this report's array nesting).

Disclosure

Please do not use this PoC against production systems you do not own or have explicit permission to test.