fix: add SSRF protection for outbound HTTP requests

[BhujArc24]

Closes #116.

Summary

Adds destination IP validation to all outbound HTTP requests issued by
unfurl, preventing SSRF attacks via user-supplied URLs and via auto-followed
oEmbed href references.

Why

Currently getPage and getRemoteMetadata pass URLs straight to
node-fetch without resolving + checking the destination. This permits
two SSRF primitives:

1. Direct SSRF — when a caller passes a user-controlled URL, an
   attacker can target loopback, private networks, or cloud metadata
   endpoints (e.g. 169.254.169.254).
2. Reflected/second-stage SSRF — even if the caller validates input,
   unfurl auto-fetches <link rel="alternate" type="application/json+oembed">
   references discovered inside fetched HTML. An attacker hosting a public
   page with a malicious oEmbed reference can pivot the request to internal
   targets without the caller's knowledge.

Changes

• New module src/ssrfGuard.ts — exports assertSafeURL(url) which:
  • Rejects non-http(s) protocols
  • Rejects URLs whose hostname (or any DNS-resolved address) falls in
    private, loopback, link-local, multicast, or reserved ranges
  • Covers IPv4 and IPv6 (including IPv4-mapped IPv6)
• src/index.ts — calls assertSafeURL before each outbound fetch
  in getPage and getRemoteMetadata.
• New allowPrivateIPs option on Opts for callers (or tests) that
  need to opt out — defaults to false.
• test/ssrf/test.ts — 29 unit + integration tests covering IP
  classification, URL validation, and end-to-end behavior.
• Existing tests updated to pass allowPrivateIPs: true since they use
  nock against localhost.

Test plan

• All existing tests pass: npx jest → 61 passed, 1 skipped (network test)
• New tests cover: IPv4 private/reserved ranges, IPv6 private/reserved
  ranges, IPv4-mapped IPv6, protocol filtering, hostname resolution,
  end-to-end unfurl rejection of internal targets

Notes

• I deliberately avoided adding a new dependency (e.g. ssrf-req-filter)
  to keep the patch minimal. The IP classification logic is small and
  testable.
• DNS is resolved once at validation time. A determined attacker could
  attempt DNS rebinding (resolving public on the validation lookup, then
  private on the actual fetch). Defending against that requires pinning
  the resolved IP and forcing node-fetch to use it — happy to follow up
  in a separate PR if you'd like that.

cc @jacktuck

[jacktuck]

Does this handle SSRF via redirection?

[BhujArc24]

Does this handle SSRF via redirection?

Good catch, no, the current PR doesn't re-validate redirect targets, which is a real gap. node-fetch follows up to opts.follow redirects by default, so an attacker-controlled public host could 302 to 169.254.169.254 and bypass the guard.

I'll update the PR to set redirect: "manual" and walk the redirect chain ourselves, calling assertSafeURL on each Location header before re-issuing. This also gives us a chance to enforce the same oEmbed-href validation on cross-origin redirects.

Will push the change shortly.

[BhujArc24]

Pushed the redirect-handling fix. New commit:

• Added safeFetch helper in src/ssrfGuard.ts that wraps node-fetch
  with redirect: "manual" and walks the redirect chain itself, calling
  assertSafeURL on each hop's target before re-issuing. Caps at
  opts.follow hops (default 20) with a clear SSRFError on overflow.
• Replaced nodeFetch with safeFetch in getPage and getRemoteMetadata.
• Added redirect tests: redirect-following works end-to-end, and the
  redirect cap fires when a loop is detected.

Full suite: 63 passed, 1 skipped (network test).

The current implementation does a fresh DNS lookup per hop, so a
redirect to a different host gets fully re-validated. There's still a
theoretical DNS-rebinding window between validation and the actual
TCP connect inside node-fetch, defending against that requires
pinning the resolved IP for the request, which I can do in a follow-up
if you'd like.

[BhujArc24]

Hey, just bumping this in case it got buried. Happy to clarify anything or tweak the PoC. No rush.

[jacktuck]

Also, did we ever actually return anything meaningful during the SSRF? Since unfurl only extracts specific details, it may not have returned anything useful to an attacker.

[BhujArc24]

Also, did we ever actually return anything meaningful during the SSRF? Since unfurl only extracts specific details, it may not have returned anything useful to an attacker.

Fair point, for the direct path, you're right that the text/html content-type check throws before parsing non-HTML responses, so vanilla AWS metadata wouldn't leak much.

The cases where it still has teeth:

1. Internal services that serve HTML (Jenkins, Kubernetes
   dashboards, admin panels), title + OG tags get returned.
2. The oEmbed path accepts application/json+oembed, so a
   fetch to internal JSON APIs (Consul, etcd, internal REST)
   gets parsed and matching keys leak through.
3. Blind SSRF via error-type differentiation for port scanning.

Severity probably depends on weighting those scenarios. I'd defer to your judgment on the CVSS.

Pushed review feedback as 3baf210.

[BhujArc24]

Hey Jack, following up on #117. Everything's review-ready: 4 checks passing, no conflicts, and I addressed all the feedback (redirect SSRF handling in fb31116, plus the review fixes in 3baf210/c3967d3).

On your severity question, here's a proposed CVSS 3.1 to save you the work:

Vector: AV:N/AC:L/PR:N/UI:N/S:C/C:L/I:N/A:N → 6.5 (Medium)

Reasoning: remotely triggerable with no auth or user interaction (the reflected oEmbed path fires without the caller's knowledge), scope changed since unfurl is used to reach internal services, and confidentiality Low rather than High because extraction is partial, title/OG tags and matching JSON keys, not full response bodies. If you weight the internal-JSON-API leakage more heavily, bumping C to High puts it around 7.7. Your call.

I'd like to get a CVE assigned for this. Cleanest path is through GitHub once you publish a security advisory, I'm happy to draft the full advisory text so all you'd need to do is review and click publish. If you'd rather not, I'll request one through MITRE next week instead.

Let me know which you prefer, and if anything's still blocking the merge I'll take care of it. Thanks!