Surfaced during Copilot review of #379 (cors-proxy).
`validateExternalUrl()` in `src/utils/ssrf.js` swallows individual record-type lookup failures with `.catch(() => [])` and only fails closed if the outer try/catch fires. The path that's not covered: both `dns.resolve4` and `dns.resolve6` succeed but return empty arrays, or both return empty due to transient DNS issues.
In that case the loop iterates over zero IPs, finds no private one, returns `{ valid: true }`. The subsequent `fetch()` then resolves the hostname via the OS resolver, which may pick up a different (possibly private) address. SSRF check is bypassed.
Reproduction sketch
- Hostname with no A/AAAA but a CNAME chain that the OS follows but Node's `dns.resolve4/6` doesn't (rare but legal)
- Forced DNS failure on resolve4/6 while the OS cache still has an entry
- Adversarial nameserver returning empty records to the validating resolver but a real address to the OS resolver
Suggested fix
In `validateExternalUrl()`, when the host is a hostname (not an IP) and DNS lookup is required, treat `addresses.length === 0 && addresses6.length === 0` as failure (return `{ valid: false, error: ... }`).
```js
const addresses = await dns.resolve4(hostname).catch(() => []);
const addresses6 = await dns.resolve6(hostname).catch(() => []);
if (addresses.length === 0 && addresses6.length === 0) {
return { valid: false, error: `No A/AAAA records for ${hostname}`, url: null };
}
```
Scope / blast radius
`validateExternalUrl` / `safeFetch` are called from:
Every consumer is by definition SSRF-sensitive, so failing closed on lookup-empty is the correct default. The risk of breaking legitimate hostnames with quirky DNS configs is small — those calls were going to fail at `fetch()` anyway.
Refs
Surfaced during Copilot review of #379 (cors-proxy).
`validateExternalUrl()` in `src/utils/ssrf.js` swallows individual record-type lookup failures with `.catch(() => [])` and only fails closed if the outer try/catch fires. The path that's not covered: both `dns.resolve4` and `dns.resolve6` succeed but return empty arrays, or both return empty due to transient DNS issues.
In that case the loop iterates over zero IPs, finds no private one, returns `{ valid: true }`. The subsequent `fetch()` then resolves the hostname via the OS resolver, which may pick up a different (possibly private) address. SSRF check is bypassed.
Reproduction sketch
Suggested fix
In `validateExternalUrl()`, when the host is a hostname (not an IP) and DNS lookup is required, treat `addresses.length === 0 && addresses6.length === 0` as failure (return `{ valid: false, error: ... }`).
```js
const addresses = await dns.resolve4(hostname).catch(() => []);
const addresses6 = await dns.resolve6(hostname).catch(() => []);
if (addresses.length === 0 && addresses6.length === 0) {
return { valid: false, error: `No A/AAAA records for ${hostname}`, url: null };
}
```
Scope / blast radius
`validateExternalUrl` / `safeFetch` are called from:
Every consumer is by definition SSRF-sensitive, so failing closed on lookup-empty is the correct default. The risk of breaking legitimate hostnames with quirky DNS configs is small — those calls were going to fail at `fetch()` anyway.
Refs