clawwork-ai / clawwork-ai/ClawWork
[Bug] safeFetch is vulnerable to DNS rebinding TOCTOU (SSRF bypass)
- Dominant language
- TypeScript
- Stars
- 532
- Forks
- 75
- Avg merge
- 5h 31m
- Merged PRs (30d)
- 1
Description
## Problem
`assertNotPrivateHost` resolves the hostname via `dns.resolve4/resolve6`, checks the resulting IPs against a private-range blocklist, and returns. Immediately after, `safeFetch` calls `net.fetch(url, ...)` with the **original URL string**, which performs its own independent DNS resolution. A DNS rebinding attacker can return a public IP on the first lookup (passing the guard) and a private IP on the second lookup (the actual fetch), completely bypassing the SSRF check. This is directly triggerable from the renderer via `artifact:save-image-url` and the auto-extract path.
## Location
**File:** `packages/desktop/src/main/net/safe-fetch.ts:12-27` + `packages/desktop/src/main/net/ssrf-guard.ts:49-59`
```typescript
// safe-fetch.ts
export async function safeFetch(url: string, opts: SafeFetchOptions = {}): Promise {
const parsed = new URL(url);
if (parsed.protocol !== 'https:') throw new Error('HTTPS required');
await assertNotPrivateHost(parsed.hostname); // ← first DNS resolution
// ...
const res = await net.fetch(url, { signal: controller.signal }); // ← second independent DNS resolution
```
Attack surface: both call sites (`packages/desktop/src/main/artifact/auto-extract.ts:35` and `packages/desktop/src/main/ipc/artifact-handlers.ts:161`) pass renderer-controlled URLs directly into `safeFetch`.
## Fix Approach
Pin the IP between the guard and the fetch so the two operations cannot disagree. Options:
1. **Preferred:** do the DNS resolution yourself in `assertNotPrivateHost`, return the resolved IP, and pass that IP to `net.fetch` via a rewritten URL (with `Host` header preserved for TLS SNI). `net.fetch` in Electron supports a `Host` header override.
2. **Alternative:** wrap `dns.lookup` with a short-lived cache keyed by hostname that both the guard and the fetch read through, so both observe the same resolution result.
Either approach closes the TOCTOU window. The current code cannot be fixed in-place without pinning.
## Verification
1. Run `pnpm check` — must pass.
2. Unit test: mock `net.fetch` to observe the resolved IP it is given; confirm it matches what the guard saw.
3. Manual: set up a rebinding test server that flips between `1.1.1.1` and `127.0.0.1` across two DNS queries; confirm `safeFetch` rejects.
## Context
- **WG:** Observability & DX (also security-critical)
- **Priority:** Medium (security, but attack surface is limited to user-supplied image URLs)
- **Estimated effort:** 1-2 hours
Contributor guide
Research direction
Start with packages/desktop/src/main/net/safe-fetch.ts and packages/desktop/src/main/net/ssrf-guard.ts, then inspect the call sites in auto-extract.ts and artifact-handlers.ts. Run pnpm check and add a unit test around net.fetch that verifies the fetched IP matches the guarded resolution; completion also requires the rebinding scenario to be rejected.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- electron, typescript
- Domain
- desktop, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100