Follow-up hardening / design items from a security review
- 主要言語
- C
- スター
- 22.3k
- フォーク
- 2.1k
- 平均マージ
- 1日 3時間
- マージ済み PR(30日)
- 4
説明
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_id`s 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.
コントリビューションガイド
評価
この issue はまだ評価されていません。