Soundcheck: security review findings

[github-actions]

Security Review

Threat Model Summary

Ziplock is a macOS-only developer tool: it wraps Claude Code in an Apple Seatbelt sandbox (sandbox_init FFI) and a localhost SOCKS5/HTTP CONNECT proxy that filters DNS via Cloudflare DoH (1.1.1.3). The only realistic adversarial actor is prompt-injected content flowing through Claude's context; the invoking user is trusted. Direct private/LAN IP connections from the proxy are intentional (ADR 001) and are not flagged.

---

Findings

Medium

#	File:Line	Skill	Finding	Fix
M1	src/dns.rs:45	cryptographic-failures	rustls-platform-verifier trusts the macOS system keychain. Any MITM root CA installed via corporate MDM can terminate the TLS session to family.cloudflare-dns.com and inject arbitrary DNS responses, completely bypassing the malware/adult-content filter. Hard-coded Cloudflare IPs prevent IP-level redirect but are irrelevant once TLS is broken by a trusted CA.	Add SPKI certificate pinning for Cloudflare's leaf/intermediate cert after handshake, or bundle Cloudflare's issuing roots (ISRG Root X1 / DigiCert Global Root G2) as an isolated trust store. Document accepted risk if pinning is omitted.
M2	src/proxy.rs:275	insecure-design	resolve_system_private() calls tokio::net::lookup_host() with no timeout. On a misconfigured or adversarial LAN the system resolver can stall for minutes, pinning the Tokio task and its semaphore permit.	Wrap in tokio::time::timeout(Duration::from_secs(3), ...).
M3	src/proxy.rs:335	insecure-design	handle_socks5() has no overall handler timeout. A client (or a prompt-injected Claude) that connects and stalls after sending one SOCKS5 byte keeps a semaphore permit occupied until the TCP stack's own timeout fires (~2+ min).	Wrap the SOCKS5 handshake phase in a timeout matching HEADER_READ_TIMEOUT. Also apply an idle timeout to the copy_bidirectional tunnel phase.
M4	src/proxy.rs:611	token-smuggling	has_cl_te_conflict() correctly detects dual CL+TE but does not strip or reject Transfer-Encoding from plain-HTTP proxy requests when no Content-Length is present. A chunked request is forwarded verbatim without decoding; if an upstream server treats the connection as persistent, chunk boundaries control next-request framing.	Set Connection: close on every forwarded plain-HTTP request to eliminate persistent-connection desync risk, or decode chunked bodies before forwarding.

---

Low

#	File:Line	Skill	Finding	Fix
L1	src/proxy.rs:466	header-injection	String::from_utf8_lossy silently replaces non-UTF-8 bytes in the HTTP request line with U+FFFD, masking the original bytes in logs and potentially obscuring crafted input from analysis.	Replace with std::str::from_utf8 and return a 400 Bad Request on non-UTF-8 input, making the rejection explicit.
L2	src/proxy.rs:184	insecure-design	No hostname length cap before DoH lookup. Arbitrarily long hostnames (up to SOCKS5 domain-field limits) are passed to hickory and written to log lines. RFC 1035 limits hostnames to 253 characters.	Add if host.len() > 253 { return Err(...); } at the top of resolve_host().
L3	src/proxy.rs:276	sensitive-disclosure	resolve_system_private() issues a plaintext UDP query to the DHCP-assigned DNS server for NXDOMAIN fallback hostnames. A prompt-injected crafted hostname (e.g. data.corp.internal) is transmitted to any LAN-visible DNS server, creating a covert exfiltration channel at ~1 lookup/connection.	Log a warn! when falling back to the system resolver (making the disclosure observable). Optionally apply a character-set/length filter before calling lookup_host.
L4	src/proxy.rs:363	insecure-design	Allowed outbound connections are logged only at DEBUG level; the default log level produces no allowed-connection audit trail, making post-incident analysis difficult.	Emit allowed connections at INFO level; keep blocked events at WARN.
L5	src/proxy.rs:14	insecure-design	MAX_CONNECTIONS=256 semaphore is shared across SOCKS5 and HTTP listeners with no per-source limit. Combined with M2/M3 (no timeouts), a single misbehaving process can exhaust all permits.	Split the semaphore between SOCKS5 and HTTP, or add an idle timeout to copy_bidirectional to reclaim permits from stalled tunnels.
L6	src/sandbox.rs:37	insecure-design	claude_supports_auto_mode() calls the claude binary and blocks with no timeout. A stalled or replaced binary hangs ziplock indefinitely before the Seatbelt sandbox or DNS proxy are applied.	Add a deadline (e.g. 5 s) via recv_timeout on a spawned thread, or use tokio::time::timeout with tokio::process::Command.
L7	src/sandbox.rs:116	injection	sanitize_sbpl_path converts non-UTF-8 OsStr paths via to_string_lossy, silently embedding U+FFFD (3-byte sequence EF BF BD) inside SBPL double-quoted string literals. Apple's sandbox parser behavior for these bytes is undocumented.	Reject paths that are not valid UTF-8: `path.to_str().ok_or_else(
L8	src/sandbox.rs:511	ipc-security	(allow mach-register (local-name-regex #".*")) permits sandboxed Claude to register any per-session Mach service name. A prompt-injected Claude could squat on names expected by in-session child tools (e.g. xcodebuild, Swift compiler helpers).	Enumerate specific service-name patterns Claude Code actually registers (via sandbox trace logs) and replace the wildcard with those patterns.
L9	src/sandbox.rs:601	race-condition	prepare_tmpdir() uses symlink_metadata() (lstat) then set_permissions() (chmod). A same-UID concurrent process could theoretically replace the directory with a symlink in the TOCTOU window.	Use `O_DIRECTORY
L10	src/sandbox.rs:635	broken-access-control	find_1password_dirs() suffix-matches lowercased directory names (.ends_with(".1password")). A pre-created ~/Library/Group Containers/attacker.1password/ directory receives an SBPL write carve-out even with no 1Password installed.	Verify a 1Password-specific marker file exists inside the matched directory (e.g. t/agent.sock) before granting the carve-out, or enforce the full bundle-ID prefix format with a regex.
L11	src/dns.rs:45	insecure-design	No explicit response-size limit on DoH replies. A MITM (enabled by M1) could inject a large response body, causing hickory to allocate a large buffer before rejecting the payload.	Configure edns0 max payload or document the accepted risk once hickory's default limit is confirmed; add a code comment.
L12	src/main.rs:240	race-condition	signal_forward() stores the child PID as a raw i32. If Claude exits and its PID is recycled before a delayed signal arrives, the signal is delivered to an unrelated process.	Set a shared AtomicBool in the wait task when wait() returns; check it in signal_forward before calling kill().
L13	Cargo.toml:10	supply-chain	fast-socks5 = "1" allows any 1.x update via cargo update. Cargo.lock currently pins 1.0.0 with a checksum, but a CI pipeline that runs cargo update would adopt a future malicious 1.x release. The crate parses untrusted hostnames from Claude.	Pin to exact version: fast-socks5 = "=1.0.0". Add cargo audit to CI.
L14	src/proxy.rs:355	ssrf	If fast-socks5 delivers a bracketed IPv6 literal [::1] as the ATYP=0x03 domain string, host.parse::<IpAddr>() fails, falls through to DoH (NXDOMAIN), then system resolver (also fails) — resulting in an opaque connection error rather than a clear "blocked" response. Not an SSRF bypass (connection is denied), but confusing.	Strip brackets before parsing: let host = host.strip_prefix('[').and_then(|s| s.strip_suffix(']')).unwrap_or(host).
L15	src/main.rs:133	insecure-local-storage	Log file opened with O_CREAT then set_permissions(0o600) applied to the fd. There is a theoretical TOCTOU between open and chmod, but ~/.claude/ is created with mode 0700, making other users unable to reach the file. Effectively unreachable.	No code change required. Optionally add a comment noting that set_permissions on a File is fd-based (fchmod) and race-free.

---

Attack Chains

Chain C1 — Medium (findings M1, L11)

An MDM-deployed MITM root CA allows an on-path corporate proxy to terminate and re-originate the TLS session to family.cloudflare-dns.com, injecting arbitrary DNS responses and completely bypassing the malware/adult-content filter. Because hickory has no response-size limit (L11), the same adversary can inject a crafted DoH reply with an abnormally large body, forcing a proportionally large buffer allocation on every DNS lookup Claude makes. The attacker walks away with full DNS filter bypass — any blocked domain (malware C2, phishing, adult content) is now reachable — plus the ability to impose memory pressure on the resolver with every query.

Chain C2 — Medium (findings M2, M3, L5)

A prompt injection causes Claude to open many concurrent SOCKS5 connections to crafted .local hostnames on an adversarial or misconfigured LAN where the system resolver stalls indefinitely. Because resolve_system_private has no timeout (M2), each Tokio task blocks forever inside lookup_host while holding its semaphore permit; because handle_socks5 has no overall handler timeout (M3), those tasks never exit. With the shared semaphore capped at 256 permits and no per-source limit (L5), 256 such stalled connections exhaust the entire pool, causing every subsequent network connection attempt by Claude to be immediately dropped — effectively denying Claude all outbound network access for the rest of the session.

Chain C3 — Low (findings L2, L3)

A prompt injection causes Claude to request crafted hostnames that encode exfiltrated data as subdomains (e.g. a base64 chunk in a .corp.internal label). When Cloudflare DoH returns NXDOMAIN for these names, resolve_system_private issues a plaintext UDP query to the LAN DHCP DNS server, making the full hostname visible to any on-path observer (L3). Because there is no hostname length cap (L2), the attacker can encode up to ~253 bytes per query, maximizing the bandwidth of this covert channel. An adversary who can both inject content into Claude's context and observe LAN DNS traffic can exfiltrate data silently without any outbound TCP connection appearing in the proxy logs.

---

Summary

20 findings across 4 source files: 4 Medium, 15 Low, 1 informational. Three viable attack chains, all Medium or lower. The most significant individual finding is M1 (corporate MITM CA bypasses the entire DNS security layer); the most operationally dangerous chain is C2 (prompt-injected DoS exhausts the proxy semaphore). No Critical findings were identified. The sandbox Seatbelt profile and core proxy filtering logic are sound; the gaps are primarily around missing timeouts, missing certificate pinning for the DoH trust anchor, and a few narrow race conditions.

---

Run /security-cleanup to apply fixes interactively. No files were modified by this review.

---

Generated by Soundcheck