Follow-up hardening / design items from a security review

[gigioneggiando]

Hi Antirez — thanks again for the go-ahead to open things here.

I emailed you a coordinated security report on ds4. The memory-corruption and DoS items from it are now individual PRs (#539–#548, one per fix, each verified under ASan/UBSan or compile-checked). This issue tracks the remaining lower-severity items, where the right fix is a judgment call I'd rather leave to you than force through a PR. Happy to open a focused PR for any of these once you've picked a direction.

Hardening (behavior/robustness)

1. HTTP request framing leniency (ds4_server.c, header_end / content_length). A bare \n\n terminator is accepted alongside \r\n\r\n; of duplicate Content-Length, the first is honoured; Transfer-Encoding: chunked is ignored. Behind a non-normalizing intermediary these are the classic request-smuggling/desync ingredients. Tightening the parser is a behavior change (could reject lenient clients), so it's your call — flagging it rather than patching it unilaterally.

2. Tokenizer token-count vs model vocab (ds4.c, vocab_load). vocab->n_vocab is taken from the tokenizer.ggml.tokens array length with no check that it matches DS4_N_VOCAB. The config path already enforces deepseek4.vocab_size == DS4_N_VOCAB, but the token array length is independent, so a GGUF can pass that config check yet ship a shorter token array. Two diagnostic paths (--first-token-test and the native blk.0 slice printer) then index the token array with logit-derived indices up to DS4_N_VOCAB — an out-of-bounds read. Suggested direction: require tokens.len == DS4_N_VOCAB in vocab_load (mirroring the existing config check), or bound the diagnostic indexing to n_vocab. I didn't send a PR because the exact invariant / load-ordering is better confirmed by you.

3. Unsalted tokenizer hash (ds4.c, hash_bytes / table_*). FNV-1a, linear probing, no resize and no per-process salt, so a hostile GGUF vocab can be precomputed offline to collide many tokens into one bucket (identical on every deployment). Low severity — it rides the same "attacker already supplies the GGUF vocab" precondition as the model-file findings, and normal loads bound insertions by n_vocab. Reported for completeness; a per-process salt would close it if you think it's worth it.

Design-level (your call on the approach)

These are the design-level findings from the emailed report — each sits behind an operator choice (agent write tools enabled, clustering turned on and reachable), so they're confinement/hardening gaps rather than default-on vulnerabilities. I'm listing them here for visibility but deliberately keeping specifics in the private report:

4. Agent write/edit tools follow symlinks (ds4_agent.c) — no realpath/O_NOFOLLOW / workspace-confinement check, so a symlink inside the intended workspace redirects a write outside it. Direction: resolve and reject out-of-workspace targets, or an explicit --allow-symlinks opt-in.
5. Distributed WORK routing dials a peer-supplied host/port (ds4_distributed.c) — structure is validated but the destination isn't, so a worker can be steered to connect to internal addresses (SSRF-shaped pivot). Direction: allow-list/CIDR of known cluster addresses, or authenticate the distributed protocol.
6. Unbounded per-session KV in distributed workers (ds4_distributed.c, dist_worker_get_session_locked) — distinct peer session_ids create unbounded context-sized sessions with no cap/eviction. Direction: a session cap + eviction.
7. CDP browser automation (ds4_web.c) launches Chrome with --remote-allow-origins=*, disabling the DevTools-protocol origin check. Direction: scope the allowed origin.

(For completeness: I left the distributed listener's opt-in receive timeout alone — the code comment explains the long-idle-KV-transfer tradeoff, so it reads as deliberate.)

Full detail — exact lines, and PoCs where they exist — is in the emailed report. Just say the word on any of these and I'll send a focused PR.