security: harden model-file parsing, server, and build (untrusted-mirror threat model)

[KingIcyCreamProjects]

What

A security-hardening pass against the model-file trust boundary (colibri loads model dirs + safetensors from untrusted mirrors), plus the server and build. Every item was adversarially confirmed against the tree, with no regression vs. legitimate GLM-5.2 loading or normal server flows.

Findings fixed

Model-file trust boundary

• glm.c: qt_check_fmt() rejects (rather than silently infers) any quantized tensor whose weight/scale byte counts disagree with the config shape, at all three format sites (qt_from_disk + both expert_load arms) — closes a matmul heap OOB read and a per-row scale OOB write (SEC-1). embed_row zero-fills out-of-range token ids (SEC-5).
• st.h: cross-check numel == nbytes/esz for float dtypes + shape-product overflow guard + st_read_f32_cap at config-sized sinks (SEC-2 / B1); st_read_slice_f32 NULL + bounds guard (B6).
• glm.c: SEC-9 — bound the config.json / ref_glm.json slurps (256 MB cap + NULL-checked alloc + full-read check); the old unbounded ftell → malloc(n+1) gave a hostile file a load-time OOM or, on malloc failure, a NULL-deref via b[got]=0.
• tok.h: negative-id + json_get NULL/type guards (SEC-3); tk_read_file fseek/ftell/malloc/fread error handling + size cap (B3 / B4).
• json.h: bound \u reads against the NUL + full-keyword strncmp for true/false/null (SEC-4); growable string buffer replacing the silent 64 KB truncation (B5).

Server (openai_server.py)

• Fail-closed on non-loopback bind without a key (SEC-6); Host-header allowlist incl. OPTIONS preflight (SEC-7); auth-gate /health & /experts telemetry (SEC-8); scheduler head-of-line fix (B2); [DONE] under ka_lock (B9).

Independent corroboration

An unrelated adversarial audit re-derived SEC-1 (matmul OOB read) and SEC-2 (scale OOB write) from scratch against a tree without these fixes — same two bugs, same locations. They're real, not theoretical.

Status / ask

This branch was cut from an older dev and is behind, so it needs a rebase onto current dev before merge. The conflicts are nearby churn — dev contains none of these fixes (verified: no qt_check_fmt / st_read_f32_cap upstream), so nothing here is superseded. Before investing in the rebase: do you want this as one PR, or split into (a) model-file C trust-boundary fixes, (b) server fixes, (c) build/docs? I'll do either. Threat model throughout is the untrusted-mirror model directory.

[KingIcyCreamProjects]

Rebased onto current dev — now 0 behind, and re-verified end-to-end.

• One rebase conflict (st.h st_read_slice_f32): resolved to keep the audit's NULL-check on the malloc'd raw while adopting dev's st_pread_full (st: chunked pread with EINTR retry and honest short-read errors #331 chunked + EINTR read) — a strict superset of the audit's pread(...)!=nb, so no protection is lost and short reads / >2 GB tensors are now handled honestly.
• Ran an adversarial re-verify of the rebased tree: every fix is present, correct, and still effective against current dev (SEC-1 qt_check_fmt at all 3 format sites, SEC-2/B1 numel-vs-nbytes + overflow + st_read_f32_cap, SEC-3/4/5, SEC-6/7/8, SEC-9 cfg_slurp, B2/B5/B6/B9). glm.c compiles clean; openai_server.py py_compile clean.

One thing the re-verify caught and I fixed here: dev's newer GET /profile route shipped unauthenticated, unlike its SEC-8 siblings /health and /experts — leaking per-turn timing/token telemetry. Added a commit gating it behind _is_authed() (mirrors /experts), so SEC-8's "telemetry behind auth" holds for the new route too.

Out-of-scope follow-up (pre-existing on dev, not a rebase artifact): the Linux io_uring load path (uring_finalize_load) infers fmt with the same ternary but never calls qt_check_fmt and has no fmt=4 handling — a 4th quant-format site the original audit didn't cover. I'd do that as its own PR since it needs Linux+io_uring testing plus real fmt=4 detection, not just a guard.

[KingIcyCreamProjects]

Correction to my previous comment — I reverted the /profile auth gate, and here's why (leaving the finding on the table for you to decide).

My re-verify flagged that dev's newer GET /profile returns per-turn timing/token telemetry with no auth gate, unlike its SEC-8 siblings /health and /experts. I gated it to match — but that broke test_profile_reports_recent_turns_without_auth, and reading it, the test constructs the server with an api key ("secret") and still expects /profile to answer without the key. So /profile being public even when a key is set is a deliberate design choice (presumably so the dashboard/monitors can poll live metrics without embedding the key) — not an oversight. That's your call, not something I should force in a rebase, so I backed it out. The branch is green again (71/71 python tests pass).

Flagging it so it's a conscious decision: when the server is exposed with COLI_API_KEY set, /profile still leaks per-turn wall_s / prompt+completion token counts / expert disk+wait+matmul / attention / lm_head / forwards to any host-allowlisted caller. _check_host covers DNS-rebinding, so it's low severity, but it's inconsistent with SEC-8's "telemetry behind auth". If you want it gated for parity, it's a 4-line change plus flipping that test to send the key — happy to do it, or leave /profile public by design. Your call.

Net: the PR is exactly the original audit (SEC-1…B9 + SEC-9), rebased clean onto dev, no behavior changes beyond the documented hardening.

[KingIcyCreamProjects]

Extended the audit after a research + code pass (corroborated against the llama.cpp GGUF integer-overflow CVEs — 33298/53630/27940/25664-68 — and the safetensors "two-authorities" TOCTOU class). The original audit hardened glm.c thoroughly but left sibling surfaces. Four fixes, all Windows-built + tested:

• CRITICAL — heap OOB write in the OLMoE loader (olmoe.c load_expert_w). The dtype-3 (U8/I8) container path wrote header-controlled byte counts into config-sized buffers (q=malloc(O*I), scale=falloc(O)). st_init deliberately skips its numel↔nbytes cross-check for dtype-3, so a crafted .safetensors with an over-declared U8 expert weight (or oversized .qs) was a controlled heap overflow — the exact SEC-1 primitive glm.c's qt_check_fmt already rejects, just never applied to olmoe. Added the matching size guard (weight == O*I bytes, scale == O elems) in the shared loader.
• olmoe.c load_cfg — unbounded config.json slurp (SEC-9 class) + json_get(...)->num NULL-deref on any missing key. Added the 256 MB cap + NULL/short-read guards, a req_num() that rejects missing/non-numeric keys, and CKR-style range checks.
• tok.h (hostile tokenizer.json) — type-check vocab ids / merge-pair entries before deref (was NULL-deref / silent id=0), and cap maxid so n_ids=maxid+1 can't signed-overflow into a multi-GB calloc. Verified the real 151K-vocab GLM tokenizer still loads + encodes unchanged.
• openai_server.py — APIHandler.timeout = 30: a slowloris client could otherwise pin a worker thread + KV slot forever, before the B2 queue bound applies.

Validation: glm.c/olmoe.c compile clean, 71/71 python tests pass, real GLM tokenizer loads, tiny oracle 32/32.

Two things deferred (not in this PR):

1. The Linux io_uring finalize path (uring_finalize_load) is a 5th fmt-inference site with the same missing qt_check_fmt + no fmt=4 handling — but it's #ifdef __linux__, uncompilable/untestable on my Windows box, so it needs its own Linux-gated PR (the fix is the proven pread-arm pattern verbatim). Flagging rather than shipping a memory-safety change I can't build.
2. /profile stays public by design (a test asserts it even with a key set) — your call whether to gate it.

[JustVugg]

Heads-up on the rebase you flagged: I did an independent audit of the model-file trust boundary and confirmed your SEC-1/SEC-2 (the safetensors OOB read/write) and SEC-9 (config slurp) against current dev — they're real and present in v1.0.0. Given the severity I've landed a minimal standalone fix for just the C memory-corruption items in #413 (numel↔nbytes cross-check + shape-overflow guard in st.h; a validating qt_resolve_fmt replacing the silent int2 fallback at all three sites; bounded config slurp). Verified no regression (TF token-exact on every quant format, fmt=4 preserved, ASan/UBSan clean, hostile file refused).

This is meant to unblock your rebase, not replace your PR: #413 is deliberately scoped to the C corruption bugs so v1.0.1 can ship them fast. The server-side hardening (SEC-6 fail-closed bind, SEC-7 Host allowlist, SEC-8 telemetry auth) and the build/docs items are all still yours and don't overlap #413. So to answer your "one PR or split?": please split — rebase #368 down to the server + build items and it lands clean on top of #413. Credited you in the #413 description; thank you for the careful writeup, it made the independent verification fast.

[JustVugg]

Heads-up: c/glm.c was just split into c/colibri.c + header modules (#391, now merged into dev). This PR touches the old glm.c, so it will need a rebase onto current dev with its changes moved into colibri.c (or the relevant extracted header: quant.h, sample.h, kv_persist.h, telemetry.h, grammar.h). The make glm target still works (alias of make colibri), so no build scripts break. Apologies for the churn — ping me if the rebase gets thorny and I can help place the hunks.