Skip to content

vocab : validate special-token ids against vocab size (fix OOB read in llama_vocab::impl::load) - #25508

Closed
Yuva1l wants to merge 1 commit into
ggml-org:masterfrom
Yuva1l:fix-vocab-eog-oob-read
Closed

Yuva1l wants to merge 1 commit into
ggml-org:masterfrom
Yuva1l:fix-vocab-eog-oob-read

Conversation

@Yuva1l

@Yuva1l Yuva1l commented Jul 10, 2026

Copy link
Copy Markdown

Summary

A default special-token id can be used as an out-of-bounds index into id_to_token during vocab load, causing a heap-buffer-overflow read.

This is the load()-path sibling of GHSA-g4cc-763q-h9h6, whose fix (c33fe8b8, #14145) hardened only print_info() (switching to .at()). The identical unchecked primitive remains in llama_vocab::impl::load().

Root cause

Each special token gets a per-tokenizer default id (e.g. special_eos_id = 2 for llama/SPM, 11 for gpt2). The clamp loop in llama_vocab::impl::load() only bounds-checks an id when its KV key is present:

if (!ml.get_key(std::get<0>(it), new_id, false)) {
    continue;                 // key absent -> default id never checked
}
if (new_id >= id_to_token.size()) { /* keep default (also unchecked) */ }

id_to_token is sized to n_tokens = gguf_get_arr_n(tokenizer.ggml.tokens), which the file controls. If the file declares a tiny vocab and omits the *_id key, the default id (e.g. 2) is never re-validated. It then flows into special_eog_ids and is dereferenced with a raw operator[] in the "printing all EOG tokens" loop:

for (auto tid : special_eog_ids) {
    auto & text = id_to_token[tid].text;   // OOB read when tid >= id_to_token.size()
    ... text.c_str() ...
}

Reproduce

A GGUF with a 2-token llama vocab and no tokenizer.ggml.eos_token_id key:

# poc.py — needs the repo's gguf-py
import gguf, numpy as np
w = gguf.GGUFWriter("oob_read_poc.gguf", "llama")
w.add_context_length(64); w.add_embedding_length(16); w.add_block_count(1)
w.add_feed_forward_length(32); w.add_head_count(2); w.add_head_count_kv(2)
w.add_layer_norm_rms_eps(1e-5)
w.add_tokenizer_model("llama")            # default special_eos_id == 2
w.add_token_list(["a", "b"])              # id_to_token.size() == 2
w.add_token_scores([0.0, 0.0]); w.add_token_types([1, 1])
# tokenizer.ggml.eos_token_id intentionally OMITTED
w.add_tensor("token_embd.weight", np.zeros((16, 2), dtype=np.float32))
w.write_header_to_file(); w.write_kv_data_to_file(); w.write_tensors_to_file(); w.close()
cmake -B build -DLLAMA_SANITIZE_ADDRESS=ON && cmake --build build -j --target llama-cli
python3 poc.py
./build/bin/llama-cli -m oob_read_poc.gguf -p x -n 1
# => AddressSanitizer: heap-buffer-overflow  READ of size 8  in llama_vocab::impl::load

Fix

After resolving each special-token id (default or explicit), validate it against id_to_token.size() and disable it (LLAMA_TOKEN_NULL) if out of range. Valid models are unaffected. With this change the PoC model loads cleanly.

Impact

Out-of-bounds heap read during model load → crash (DoS) when loading an untrusted GGUF; the OOB std::string deref can additionally read adjacent heap. Covered scope (src/**). Severity comparable to the sibling GHSA-g4cc (Medium).

special_eos_id (and the other special-token ids) get a per-tokenizer default
value. When the corresponding tokenizer.ggml.*_id KV key is absent, the default
was never validated against id_to_token.size(); an out-of-range default then
reaches id_to_token[tid] in the special_eog_ids loop as an out-of-bounds read.
Validate each resolved id after the KV loop and disable it if it is out of range.

This is the load()-path sibling of GHSA-g4cc-763q-h9h6, whose fix (c33fe8b)
hardened only print_info().
@Yuva1l
Yuva1l marked this pull request as ready for review July 10, 2026 00:10
@Yuva1l
Yuva1l requested a review from CISC as a code owner July 10, 2026 00:10
@ggml-gh-bot

ggml-gh-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

Hi @Yuva1l, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • PR Template not respected: Please respect the template when creating a new pull request. Make sure to fill out all required sections.

  • AI-generated content: This project does not accept PRs, descriptions or commit messages that are fully or predominantly AI-generated. If you have used AI to assist you in writing code, please make sure to disclose that explicitly.


Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@CISC

CISC commented Aug 3, 2026

Copy link
Copy Markdown
Member

Thank you for the report.

@CISC CISC closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants