Skip to content

fix: harden SSRF protection against DNS rebinding#181

Merged
ospfranco merged 2 commits into
OP-Engineering:mainfrom
byndnmz:fix/dns-rebinding-ssrf
Jun 23, 2026
Merged

fix: harden SSRF protection against DNS rebinding#181
ospfranco merged 2 commits into
OP-Engineering:mainfrom
byndnmz:fix/dns-rebinding-ssrf

Conversation

@byndnmz

@byndnmz byndnmz commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

This PR hardens SSRF protection by binding outbound fetches to the resolved and validated address and applying the same protection to manual redirect targets.

It also strengthens local/private address validation for normalized IPv6 and embedded IPv4 forms.

Details are intentionally kept minimal while the security advisory is still in draft.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens SSRF protections in getLinkPreview() by validating resolved IP addresses more robustly (including embedded IPv4-in-IPv6 forms) and by pinning outbound requests to the previously resolved/validated address to mitigate DNS rebinding, including for manual redirect targets.

Changes:

  • Add undici and introduce a pinned Agent/dispatcher to bind fetches to the validated resolved IP.
  • Replace regex-only loopback/private checks for resolved addresses with node:net BlockList checks plus IPv6 embedded-IPv4 detection.
  • Add Jest coverage for pinned dispatchers and additional IPv6 normalization edge cases.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

File Description
package.json Adds undici dependency required for pinned dispatchers.
bun.lock Locks the new undici dependency.
index.ts Implements IP blocklisting, embedded-IPv4 detection, pinned dispatchers, and dispatcher cleanup; applies redirect pinning.
tests/index.spec.ts Adds tests for dispatcher pinning and normalized IPv6/local blocking.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread index.ts Outdated
Comment thread index.ts Outdated
Comment thread index.ts Outdated
Comment thread __tests__/index.spec.ts
@ospfranco

Copy link
Copy Markdown
Collaborator

Adding undici requires node? Then this will break React Native compatibility?

@byndnmz

byndnmz commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Good point. I updated the PR to avoid introducing a Node-only dependency.

The latest version removes the undici dependency and does not use node:net / BlockList, so it should not add a new Node-specific runtime requirement for React Native/mobile environments.

I also addressed the Copilot review items by adding normalization/coverage for bracketed IPv6 and zone-id local addresses, and by keeping redirected fetches on the same timeout/error handling path.

Re-tested locally:

  • npm run build
  • targeted Jest tests for resolved-address fetch and local/private address blocking

Both passed.

Also, if you agree with the advisory scope, could you please request a CVE ID from GitHub before publishing the advisory? If this should update CVE-2026-43897 instead of receiving a new CVE, that is also fine from my side.

@ospfranco ospfranco merged commit 4bb416d into OP-Engineering:main Jun 23, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants