security: six advisories — loader bounds, router NaN, serve limits - #841
Merged
Conversation
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 #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>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes six advisories reported privately. Every one was confirmed against this tree before anything was touched, and two more were re-checked and found already closed.
Reported by @zh-Processor, with exact source locations, ASAN traces and working reproducers — including honest scope notes where the reachable surface was narrower than it first looked.
inkling:
load_scalar()wrote a file-sized tensor into a 4-byte stack slotst_read_f32validates the header against itself —numel*esz == nbytes— and cannot know the destination size.float vis four bytes. A snapshot declaringgate.global_scaleas 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_capalready existed for exactly this and takes the caller's capacity. A scalar's capacity is 1.inkling:
load_cfg()indexedlayer_types[]by layer count, not array lengthBoth loops ran to
num_hidden_layerswhile indexing a JSON array of an unrelated, equally attacker-chosen length. 66 layers with a one-elementlayer_typesread past amalloc'd array that starts at capacity 8, then dereferenced whatever followed as a string. config.json alone was enough — no weights, no valid snapshot.A short array now means "unspecified", falling through to the same default an absent key uses. Element types are checked too:
kids[i]->stris only a valid pointer whent == J_STR.SERVE:
max_tokwas the one submit field nobody validatedprompt_reject()asksnp + want > ctx_max. A negativewantmakes that sum smaller thannp, so the context check passed for any prompt,kv_alloccame 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— the reporter said so himself. But the SERVE protocol is public and anything bridging it exposes it, so the check belongs beside the oneplenalready had.Router: all-NaN logits left the top-k pick at
-1, and-1was used as an indexNaN > bvis false for everybv, sobeststayed-1and becamescore[-1],usage[-1]++, and apreadat a negative offset. Release builds did not fault: they finished and returned wrong numbers over quietly corrupted memory, which is worse than a crash.colibri.chas guarded this sincetest_logit_nan.cwas written.inkling.c,kimi_k3.candolmoe.cnever received it — the recurring shape of defects in this tree, a fix that lands in one engine and not its siblings. So the guard goes inroute_trace.h, the one header all four already include, instead of being pasted a third and fourth time./profileserved telemetry to anyone, with--api-keyset/healthand/expertsgained an_is_authed()gate;/profile, served in the same block beforerequire_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: Bearerto/profileexactly 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
timeoutthat is per socket operation — so a byte every 29 s renewed it indefinitely. The code's own 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_responsehands 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.
Verified, not assumed
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.
No hostile model files were built or committed. The regressions exercise the guards directly — that is enough to prove a fix works, and it keeps exploit artifacts out of the repository.
Already fixed, re-checked
GHSA-4gw4-j89j-4c8r(@aeonframework) andGHSA-wc4x-3786-cxh7(@ajmeese7) describe primitives that #413 closed:st.hnow validatesdata_offsetsordering, file bounds andnumel*esz == nbyteswith a shape-overflow guard, andtok.hrejects negative and implausible token ids. Both were correct when filed.