[Bug] safeFetch is vulnerable to DNS rebinding TOCTOU (SSRF bypass)

[samzong]

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

// safe-fetch.ts
export async function safeFetch(url: string, opts: SafeFetchOptions = {}): Promise<Buffer> {
  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

[github-actions]

Hi @samzong,
Thanks for opening an issue!
We will look into it as soon as possible.

Details

Instructions for interacting with me using comments are available here.
If you have questions or suggestions related to my behavior, please file an issue against the gh-ci-bot repository.

[snakefood3232]

The DNS rebinding TOCTOU between assertNotPrivateHost and net.fetch is a critical SSRF bypass. I'd fix this by having assertNotPrivateHost return the resolved IP and then rewrite the URL for net.fetch with the Host header preserved. I can implement this with a PR ready for review today—I've patched similar DNS pinning vulnerabilities in Electron backends before.

[snakefood3232]

The DNS rebinding TOCTOU in assertNotPrivateHost is a critical SSRF bypass. I'd fix this by having the guard resolve and return the IP, then rewrite the URL for net.fetch while preserving the Host header. This pins the IP and eliminates the race. I can implement this with a PR ready for review today—I've patched similar Electron SSRF vulnerabilities in production desktop apps.