Skip to content

security: reject malformed model tensors at the untrusted-mirror boundary (C half of #368) - #413

Merged
JustVugg merged 1 commit into
devfrom
p-sec-trustboundary
Jul 19, 2026
Merged

security: reject malformed model tensors at the untrusted-mirror boundary (C half of #368)#413
JustVugg merged 1 commit into
devfrom
p-sec-trustboundary

Conversation

@JustVugg

@JustVugg JustVugg commented Jul 19, 2026

Copy link
Copy Markdown
Owner

What

Minimal, standalone fix for the C model-file trust boundary — the memory-safety half of #368. Colibri loads model dirs + safetensors from mirrors it doesn't control, so the file's declared shapes and byte spans are attacker-influenced input.

Findings fixed (all present in v1.0.0, independently verified against the tree)

  1. Heap OOB read + write in st_read_f32 (st.h) — the serious one. numel comes from the tensor's shape, nbytes from its offsets; no cross-check. A crafted tensor with an inflated shape made the BF16/F16 loop read past the malloc'd raw buffer and the F32 memcpy write past the caller's config-sized destination. Now numel*esz == nbytes is enforced before any copy.
  2. Integer overflow in the shape product (st.h) — numel *= shape[k] could wrap int64 to a small/negative value that then passes the cross-check. Each multiply is now guarded.
  3. Silent int2 fallback → matmul OOB (glm.c) — the ?1:?2:3 fmt inference (duplicated at three sites) fell to int2 for any unrecognized weight byte count, so a too-short weight became a valid int2 whose matmul read O*I nibbles past the buffer, and an oversized scale array overflowed the per-row t->s. Replaced by one qt_resolve_fmt() that validates weight and scale byte counts against [O,I] or refuses.
  4. Unbounded config slurp (glm.c) — ftell → malloc(n+1) gave a hostile config.json a load-time OOM, and b[got]=0 was a NULL-deref on malloc failure. Capped at 256 MB + NULL-checked.

Why not just wait for #368

#368 is the right full pass (server + build too) but needs a rebase and a split decision. This lands the memory-corruption items now, as a v1.0.1 candidate, with the smallest possible surface. The server-side items (fail-closed bind, Host allowlist, telemetry auth) still come via #368's rebase — no overlap.

Verification

  • No regression: TF token-exactness byte-identical to the pre-change binary on every quant format — full-precision 32/32, int4 11/32, int2 1/32, mix 5/32 (the lossy numbers are intrinsic to the tiny random model, not this change).
  • fmt=4 grouped preserved: the scale check is by construction the same condition detect_group_size already imposes.
  • Hostile input refused cleanly: a hand-crafted safetensors with an inflated shape exits with a clear message instead of corrupting the heap.
  • ASan + UBSan clean on both legit and hostile loads (only the pre-existing, documented one-time startup leaks remain).

Refs #368

🤖 Generated with Claude Code

@JustVugg @claude
security: reject malformed model tensors at the untrusted-mirror boun…
72e3677 
…dary

Colibri loads model directories and safetensors from mirrors it does not
control, so the file's declared shapes and byte spans are attacker-influenced
input. Three memory-safety holes on that boundary, independently confirmed
(incl. a from-scratch adversarial audit that re-derived the same two) and
present in the shipped v1.0.0:

- st.h st_read_f32: numel came from the shape, nbytes from the offsets, with
  no cross-check. A crafted tensor whose shape inflates numel past nbytes made
  the BF16/F16 loop read past the malloc'd raw buffer and the F32 memcpy write
  past the caller's config-sized destination (heap OOB read + write). Now
  enforce numel*esz == nbytes before any copy.
- st.h header parse: the shape product could overflow int64 to a small/negative
  numel that would then pass the cross-check. Guard each multiply.
- glm.c qt_resolve_fmt (new, replaces the three duplicated "?1:?2:3" fmt sites
  in qt_from_disk and both expert_load arms): the old inference SILENTLY fell
  to int2 for any unrecognized weight byte count, so a too-short weight became
  a valid int2 whose matmul read O*I nibbles past the buffer; and an oversized
  scale array overflowed the per-row t->s. Now the weight bytes must match a
  known int8/int4/int2 layout and the scale array must match the expected
  per-row (O) or grouped (O*ng) cardinality, else refuse.
- glm.c config/generation_config slurp: unbounded ftell -> malloc(n+1) gave a
  hostile file a load-time OOM, and on malloc failure b[got]=0 was a NULL
  deref. Cap at 256 MB and NULL-check.

Verified: TF token-exactness unchanged on every quant format (full-precision
32/32, int4 11/32, int2 1/32, mix 5/32 -- byte-identical to the pre-change
binary); fmt=4 grouped path preserved (the scale check is by construction the
same condition detect_group_size already imposed); a hand-crafted hostile
safetensors is refused cleanly; ASan+UBSan clean on legit and hostile loads
(only the pre-existing intentional startup leaks remain).

These are the C trust-boundary items of #368, landed as a minimal standalone
fix; the server-side and build items of that PR follow via its rebase.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 19, 2026
Merged
Merged
@JustVugg
JustVugg merged commit 3ffe4bb into dev Jul 19, 2026
8 checks passed
@JustVugg JustVugg mentioned this pull request Jul 20, 2026
Merged
fabio-rovai added a commit to fabio-rovai/colibri that referenced this pull request Jul 20, 2026
@fabio-rovai @claude
int3-g64 (fmt=5): per-group-scale 3-bit weight format — engine, conve…
58a2ac2 
…rter, tests

New weight format: int3 with ONE f32 scale per 64-input group (3.5 bits/weight
effective). Per group: 16B low plane (2 bits/val, int2 layout) + 8B high plane
(1 bit/val), values [-4,3] stored v+4. Same quantization math as
tools/quant_ablation.py _quant_last_dim(bits=3, group=64) from JustVugg#132, whose
OLMoE ablation measured int3-g64 BEATING the shipped per-row int4 on quality
(-7.5 vs -9.3pp) at ~14% fewer bits.

Engine (placed per the JustVugg#391 split): matmul_i3 + pack_int3_g64 + I3_* layout
helpers in quant.h next to their kernel family; fmt=5 branches in colibri.c's
qt_bytes/qt_alloc/qt_fill/matmul_qt/embed_row/qt_addrow/qt_matvec_rows.

Format detection now goes through JustVugg#413's qt_resolve_fmt: fmt=5 registers its
distinct weight-byte layout O*ceil(I/64)*24 and its scale cardinality
O*ceil(I/64) there, validated against [O,I] like every other format. int3-g64
and grouped-int4-at-gs=64 carry the SAME scale count, so the weight bytes are
the int3 tag; row formats keep precedence for the small-I shapes where byte
counts coincide. The io_uring expert path still used the raw ?1:?2:3 byte
inference (it missed fmt=4 grouping entirely and never set gs) — converted to
qt_resolve_fmt like the other two expert paths.

Backends: qt_cuda_upload returns 0 for fmt=5 (tensor stays CPU-side, the
documented fallback), the dense CUDA matmul gate excludes fmt=5, and Metal's
existing fmt gates (gemm fmt<=3, moe fmt 1/2) already reject it.

Converter: quant_int3_g64 in convert_fp8_to_int4.py; --ebits 3/--xbits 3 now
emit it (previously bits=3 silently produced int4).

Tests: tests/test_int3.c (bit-exact pack/unpack vs reference, matmul_i3 vs
dequant-matmul incl. short tail groups and the real GLM I=7168, QT plumbing,
qt_resolve_fmt disambiguation incl. the same-scale-count fmt=4/fmt=5 pair,
outlier-rows RMS: int3-g64 3.3x lower error than per-row int4),
tests/test_int3_load.c (hand-rolled .safetensors fixture through st_init +
qt_from_disk: fmt=5 detected and loaded next to an int4 control tensor),
tests/test_int3_convert.py (NumPy pack round-trip vs independent decoder).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fabio-rovai added a commit to fabio-rovai/colibri that referenced this pull request Jul 20, 2026
@fabio-rovai @claude
int3-g64 (fmt=5): per-group-scale 3-bit weight format — engine, conve…
5e42e70 
…rter, tests

New weight format: int3 with ONE f32 scale per 64-input group (3.5 bits/weight
effective). Per group: 16B low plane (2 bits/val, int2 layout) + 8B high plane
(1 bit/val), values [-4,3] stored v+4. Same quantization math as
tools/quant_ablation.py _quant_last_dim(bits=3, group=64) from JustVugg#132, whose
OLMoE ablation measured int3-g64 BEATING the shipped per-row int4 on quality
(-7.5 vs -9.3pp) at ~14% fewer bits.

Engine (placed per the JustVugg#391 split): matmul_i3 + pack_int3_g64 + I3_* layout
helpers in quant.h next to their kernel family; fmt=5 branches in colibri.c's
qt_bytes/qt_alloc/qt_fill/matmul_qt/embed_row/qt_addrow/qt_matvec_rows.

Format detection now goes through JustVugg#413's qt_resolve_fmt: fmt=5 registers its
distinct weight-byte layout O*ceil(I/64)*24 and its scale cardinality
O*ceil(I/64) there, validated against [O,I] like every other format. int3-g64
and grouped-int4-at-gs=64 carry the SAME scale count, so the weight bytes are
the int3 tag; row formats keep precedence for the small-I shapes where byte
counts coincide. The io_uring expert path still used the raw ?1:?2:3 byte
inference (it missed fmt=4 grouping entirely and never set gs) — converted to
qt_resolve_fmt like the other two expert paths.

Backends: qt_cuda_upload returns 0 for fmt=5 (tensor stays CPU-side, the
documented fallback), the dense CUDA matmul gate excludes fmt=5, and Metal's
existing fmt gates (gemm fmt<=3, moe fmt 1/2) already reject it.

Converter: quant_int3_g64 in convert_fp8_to_int4.py; --ebits 3/--xbits 3 now
emit it (previously bits=3 silently produced int4).

Tests: tests/test_int3.c (bit-exact pack/unpack vs reference, matmul_i3 vs
dequant-matmul incl. short tail groups and the real GLM I=7168, QT plumbing,
qt_resolve_fmt disambiguation incl. the same-scale-count fmt=4/fmt=5 pair,
outlier-rows RMS: int3-g64 3.3x lower error than per-row int4),
tests/test_int3_load.c (hand-rolled .safetensors fixture through st_init +
qt_from_disk: fmt=5 detected and loaded next to an int4 control tensor),
tests/test_int3_convert.py (NumPy pack round-trip vs independent decoder).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JustVugg added a commit that referenced this pull request Jul 20, 2026
@JustVugg
Merge #168: int3-g64 (fmt=5) per-group-scale 3-bit expert quant
f42adfa 
int3-g64 (fmt=5) container: 3-bit weights with per-group (gs=64) scales. Measured 3.34x lower outlier-row RMS than int4-row at ~25% fewer bytes (#132 ablation point). Rebased by @fabio-rovai onto post-#426 dev; fmt=5 registered in qt_resolve_fmt (#413 security gate) with fmt-4/fmt-5 disambiguation tested at real GLM I=7168; io_uring expert path converted to qt_resolve_fmt (fixes latent fmt-4 mis-tag); CUDA graceful CPU-fallback. Verified locally: no regression on existing formats, test_int3 + CI green.
JustVugg added a commit to KingIcyCreamProjects/colibri that referenced this pull request Jul 21, 2026
@JustVugg @claude
Merge dev into security branch: resolve JustVugg#391/JustVugg#413-era…
cc7461f 
… conflicts + fix fmt=5 scale cap

Conflict resolution (13 hunks, 6 files):
- colibri.c / st.h: dev already carries an equal-or-stronger guard for the same
  threats (qt_resolve_fmt resolves AND validates the quant layout, refusing
  unknown byte counts instead of falling through to int2; the shape-product
  overflow guard is present). Took dev's side — no check is lost.
- olmoe.c: the load_expert_w hardening targeted a function the olmoe refactor
  replaced; dev now validates the equivalent path in load_expert_merged.
- web/*: i18n churn only, took dev's strings.

This PR's unique value is preserved and is the point of the merge: the server
hardening in openai_server.py (+121), plus json.h, tok.h, Makefile and the
downloader/requirements pinning.

Bug found and fixed while validating: st_read_f32_cap's call site bounded the
scale read with the PER-ROW cardinality (O), which is correct for int8/int4/int2
but rejects grouped formats — int3-g64 (fmt=5) keeps O*i3_groups(I) scales, so a
legitimate container was refused (tests/test_int3_load failed). The cap now
matches the cardinality qt_resolve_fmt already validates and falloc reserved.

Verified: clean build, token-exact unchanged (fp 32/32, int4 21/32),
tests/test_int3_load ok, full make check OK.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@JustVugg
JustVugg deleted the p-sec-trustboundary branch July 24, 2026 06:53
This was referenced Aug 1, 2026
Merged
Merged
khalilswdp pushed a commit to khalilswdp/colibri that referenced this pull request Aug 5, 2026
@JustVugg @claude
security: six advisories — loader bounds, router NaN, serve limits
edcdd4e 
Fixes GHSA-pmq2-6f2p-hjvf, GHSA-c84r-hmpc-qg8h, GHSA-2h73-p2rc-4796,
GHSA-5xpg-vw35-2687, GHSA-rfqv-4g4x-j4vr and GHSA-25w8-8c74-g9c8.

Reported by @zh-Processor with exact source locations, ASAN traces and
working reproducers. Every one was confirmed against this tree before it
was touched.

## inkling: load_scalar() wrote a file-sized tensor into a 4-byte stack slot

st_read_f32 validates the header against itself -- numel*esz == nbytes --
and cannot know the destination size. `float v` is four bytes; a snapshot
declaring gate.global_scale as F32 [4096] therefore dropped 16 KiB of
attacker bytes over the frame and its return address, with RSP fully
controlled in the reported crash.

st_read_f32_cap already existed for exactly this and takes the caller's
capacity. A scalar's capacity is 1.

## inkling: load_cfg() indexed layer_types[] by layer count, not array length

Both loops ran to num_hidden_layers while indexing a JSON array of an
unrelated, equally attacker-chosen length. 66 layers with a one-element
layer_types read past a malloc'd array that starts at capacity 8, then
dereferenced whatever followed as a string. config.json alone was enough.
A short array now means "unspecified", falling through to the same default
an absent key uses; element types are checked, because kids[i]->str is only
a valid pointer when t == J_STR.

## SERVE: max_tok was the one submit field nobody validated

prompt_reject() asks `np + want > ctx_max`. A negative want makes that sum
smaller than np, so the context check passed for any prompt, kv_alloc came
out shorter than the prompt, and prefill wrote past the K/V cache. The
official gateway forces a positive integer, so this is unreachable through
openai_server.py -- but the SERVE protocol is public and anything bridging
it exposes it, so the check belongs beside the one plen already had.

## Router: all-NaN logits left the top-k pick at -1, and -1 was used as an index

NaN > bv is false for every bv, so best stayed -1 and became score[-1],
usage[-1]++ and a pread at a negative offset. Release builds did not fault:
they finished and returned wrong numbers over quietly corrupted memory.

colibri.c has guarded this since test_logit_nan.c was written. inkling,
kimi_k3 and olmoe never received it -- the recurring shape of defects here,
a fix that lands in one engine and not its siblings. So the guard goes in
route_trace.h, the one header all four already include, instead of being
pasted a third and fourth time.

## /profile served telemetry to anyone, with --api-key set

/health and /experts gained an _is_authed() gate; /profile, served in the
same block before require_auth(), did not. It carries prompt and completion
token counts and per-phase timings for the last 120 turns.

A test asserted the old behaviour, so this was deliberate once. It is not a
client requirement: the dashboard sends Authorization: Bearer to /profile
exactly as it does to the other two (web/src/lib/api.ts). The test now
asserts the gate instead, and says why.

## Serve: unbounded threads, and a timeout a drip could renew forever

One thread per connection with no ceiling, 8 MiB of stack each, and a
`timeout` that is per socket operation -- so a byte every 29 s renewed it
indefinitely. The comment claimed it stopped exactly that.

  - MAX_CONNECTIONS (64): the accept loop is bounded. Over the cap we close
    rather than queue, so a flood costs the attacker's socket, not our
    address space.
  - MAX_CONNECTIONS_PER_IP (8): a global cap alone converts exhaustion into
    starvation. Measured that while testing -- one source held every slot
    and a legitimate client was refused -- so one address is bounded well
    under the server cap.
  - READ_DEADLINE (30 s): cumulative, accept to end of body. Every read
    shrinks the socket timeout to the time left, so a drip runs the clock
    down instead of resetting it. It covers the read phase only:
    send_response hands the socket back to the ordinary timeout, because a
    600-second generation is normal and must not inherit a header clock.

All three are env-overridable (COLI_MAX_CONNECTIONS,
COLI_MAX_CONNECTIONS_PER_IP, COLI_READ_DEADLINE).

Verified under a real slowloris: 40 dripping connections from one address
hold 4 of 16 slots, a client from another address still gets 200 OK during
the attack, and every connection is reclaimed when the deadline expires.

## Not fixed here, because they already are

GHSA-4gw4-j89j-4c8r and GHSA-wc4x-3786-cxh7 describe primitives that JustVugg#413
closed: st.h now validates data_offsets ordering, file bounds and
numel*esz == nbytes with a shape-overflow guard, and tok.h rejects negative
and implausible token ids. Both re-checked against this tree.

113 server tests pass, including three new ones for the connection limits;
all four engines build clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JustVugg JustVugg mentioned this pull request Aug 5, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@JustVugg