Skip to content

gguf-py : add size guards to GGUFReader - #27188

Merged
ggerganov merged 4 commits into
ggml-org:masterfrom
oakkaya:gguf-py-size-guards
Aug 19, 2026
Merged

ggerganov merged 4 commits into
ggml-org:masterfrom
oakkaya:gguf-py-size-guards

Conversation

@oakkaya

@oakkaya oakkaya commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Overview

GGUFReader (gguf-py) currently trusts string and array length fields in
crafted GGUF files, which can cause an infinite loop with unbounded memory
growth. Verified live: a 43-byte GGUF with an array length of 2^63 hangs
gguf==0.19.0 on PyPI (no return within 20s, RAM grows ~120MB/s).

This PR adds the same limits that the C++ parser already has
(GGUF_MAX_STRING_LENGTH / GGUF_MAX_ARRAY_ELEMENTS from gguf.h) to the
Python reader:

  • constants.py: add GGUF_MAX_STRING_LENGTH and GGUF_MAX_ARRAY_ELEMENTS
  • gguf_reader.py: reject string length, array length, tensor count, and
    KV count above the limits with a fast ValueError; also reject strings
    that exceed the remaining file size (parity with the C++ loader)

Additional information

Crafted variants verified: kv_count=2^63, tensor_count=2^63, string_len=2^63,
string exceeding file size, nested array length 2^62 - all raise ValueError
immediately; a valid GGUF still loads. tests/test_gguf_reader_validation.py
and existing gguf-py tests pass (7/7).

Note: the PyPI gguf package (0.19.0, built from this directory) is still
affected; a new release will be needed after merge.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES - AI-assisted (drafted code and description; reviewed and verified by me)

Guard kv_count, tensor_count, string length, and array length
against crafted values that cause unbounded allocation or hangs.

Assisted-by: opencode
@oakkaya
oakkaya requested a review from CISC as a code owner August 16, 2026 16:16
@CISC

CISC commented Aug 17, 2026

Copy link
Copy Markdown
Member

This PR adds the same limits that the C++ parser already has (GGUF_MAX_STRING_LENGTH / GGUF_MAX_ARRAY_ELEMENTS from gguf.h) to the Python reader:

They are not part of gguf.h, just arbitrary limits defined in gguf.cpp, do the same here (move them into gguf_reader.py).

When no_alloc=true, gguf_init_from_reader accepted files where the
tensor data section (computed from header claims) exceeded the remaining
file size. This allowed crafted GGUF files to pass validation while
having insufficient data, leading to OOB reads when the loader later
mapped tensor data from the file.

Assisted-by: opencode
Per review feedback, the limits are not part of gguf.h but are
arbitrary limits defined in gguf.cpp, so define them locally in
the reader instead of exporting them from constants.

Assisted-by: opencode
@github-actions github-actions Bot added the ggml changes relating to the ggml tensor library for machine learning label Aug 17, 2026
@oakkaya

oakkaya commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Done. Moved the limits into gguf_reader.py as module-level constants. Removed from constants.py.

@CISC CISC left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can revisit gguf.cpp changes in its own PR if you like.

Comment thread ggml/src/gguf.cpp Resolved
Comment thread ggml/src/gguf.cpp Resolved
@CISC
CISC removed the request for review from JohannesGaessler August 18, 2026 13:29
@CISC CISC added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Aug 18, 2026
@ggerganov
ggerganov merged commit 0329fcd into ggml-org:master Aug 19, 2026
6 checks passed
@oakkaya

oakkaya commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor Author

@CISC This PR closes a DoS gap in the gguf-py reader: a crafted GGUF (43 bytes, array length 2^63) hangs the reader with unbounded memory growth (~120MB/s), and the PyPI gguf package (0.19.0, built from this directory) is still affected until a new release - an attacker who merely distributes a malicious GGUF can stall any client/loader that parses it. Given the earlier loader CVEs on this project (e.g. CVE-2026-27940), would you consider requesting a CVE/GHSA for this one as well ?

@CISC

CISC commented Aug 21, 2026

Copy link
Copy Markdown
Member

Given the earlier loader CVEs on this project (e.g. CVE-2026-27940), would you consider requesting a CVE/GHSA for this one as well ?

See #26892 (comment)

thecodacus pushed a commit to thecodacus/llama.cpp that referenced this pull request Sep 7, 2026
* gguf-py : add size guards to GGUFReader

Guard kv_count, tensor_count, string length, and array length
against crafted values that cause unbounded allocation or hangs.

Assisted-by: opencode

* gguf : validate tensor data section fits within file

When no_alloc=true, gguf_init_from_reader accepted files where the
tensor data section (computed from header claims) exceeded the remaining
file size. This allowed crafted GGUF files to pass validation while
having insufficient data, leading to OOB reads when the loader later
mapped tensor data from the file.

Assisted-by: opencode

* gguf-py : move size limits into gguf_reader.py

Per review feedback, the limits are not part of gguf.h but are
arbitrary limits defined in gguf.cpp, so define them locally in
the reader instead of exporting them from constants.

Assisted-by: opencode

* remove gguf.ccp changes

---------

Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@huggingface.co>
zbrad pushed a commit to zbrad/llama.cpp that referenced this pull request Sep 10, 2026
* gguf-py : add size guards to GGUFReader

Guard kv_count, tensor_count, string length, and array length
against crafted values that cause unbounded allocation or hangs.

Assisted-by: opencode

* gguf : validate tensor data section fits within file

When no_alloc=true, gguf_init_from_reader accepted files where the
tensor data section (computed from header claims) exceeded the remaining
file size. This allowed crafted GGUF files to pass validation while
having insufficient data, leading to OOB reads when the loader later
mapped tensor data from the file.

Assisted-by: opencode

* gguf-py : move size limits into gguf_reader.py

Per review feedback, the limits are not part of gguf.h but are
arbitrary limits defined in gguf.cpp, so define them locally in
the reader instead of exporting them from constants.

Assisted-by: opencode

* remove gguf.ccp changes

---------

Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@huggingface.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conversion ggml changes relating to the ggml tensor library for machine learning merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants