Skip to content

fix: pin validated upstream IPs to defeat DNS rebinding - #1414

Open
KingYuss wants to merge 3 commits into
CalloraOrg:mainfrom
KingYuss:security/issue-1261-pin-resolved-upstream-addresses-to-defeat-dns
Open

KingYuss wants to merge 3 commits into
CalloraOrg:mainfrom
KingYuss:security/issue-1261-pin-resolved-upstream-addresses-to-defeat-dns

Conversation

@KingYuss

Copy link
Copy Markdown

Overview

This PR closes the DNS rebinding gap in upstream validation. Previously validateResolvedUpstreamTarget resolved a hostname and checked the resulting addresses, but the subsequent fetch performed its own independent lookup — so an attacker-controlled DNS name could return a public IP to the validation check and a private/loopback IP (e.g. 127.0.0.1) to the actual connection. This PR makes the connection use the exact IP that passed validation, while preserving the original hostname for TLS/SNI and the Host header.

Related Issue

Changes

🔒 Pinned-Address Upstream Validation

  • [MODIFY] src/lib/upstreamTarget.ts

    • Validation now returns the resolved, vetted address(es) alongside the target so callers can pin the connection to them.
    • Adds a custom undici Agent whose connect/lookup hook returns the already-validated address instead of triggering a fresh DNS resolution.
    • The original hostname is retained for SNI and the Host header, so TLS hostname verification still targets the real host.
    • The lookup hook re-checks the address against the private-range blocklist as defense-in-depth, so a resolver that returns different addresses on successive lookups cannot reach a blocked range.
  • [MODIFY] src/routes/proxyRoutes.ts

    • Proxy dispatch uses the pinned agent so the outbound connection reuses the validated IP rather than re-resolving.
  • [MODIFY] src/webhooks/webhook.dispatcher.ts

    • Webhook dispatch applies the same pinned-address approach for parity with the proxy path.

Verification Results

npm test -- src/__tests__/proxy.integration.test.ts
Acceptance Criteria Status
The connection uses the exact IP that passed validation ✅ Pinned agent's lookup returns the validated address
A resolver returning different addresses on successive lookups cannot reach a blocked range ✅ Lookup hook re-validates against the blocklist
TLS hostname verification still uses the original host ✅ Hostname preserved for SNI / Host header
A test with a stubbed lookup demonstrates the protection ✅ Stubbed lookup in proxy.integration.test.ts

Security & Failure-Mode Handling

  • Rebinding defense: the connection is pinned to the validated IP, so a second lookup cannot swap in a blocked address.
  • Defense-in-depth: the lookup hook re-checks the address against the private-range blocklist even when pinned.
  • TLS integrity: SNI and Host continue to use the original hostname, so certificate verification is unaffected.
  • Failure mode: if validation rejects the address, the request fails closed before any connection is attempted.

Closes #1261

@drips-wave

drips-wave Bot commented Sep 30, 2026

Copy link
Copy Markdown

@KingYuss Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Thanks for the contribution! We reviewed this PR while merging the open queue and couldn't merge it yet. Here's what needs fixing:

  • It contains a syntax error and imports exports that don't exist.
  • It depends on undici, which isn't in package.json.

This branch also has merge conflicts with main. Please update it with the latest main, resolve the conflicts, fix the points above, and push — then we can merge it.

…dresses-to-defeat-dns

Resolves merge conflicts against CalloraOrg/Callora-Backend@1518ce6 (68 commit(s) behind) so the PR is mergeable.
@KingYuss

KingYuss commented Oct 5, 2026

Copy link
Copy Markdown
Author

@greatest0fallt1me thanks for the review — pushed a fix to this branch.

What changed:

  • src/lib/upstreamTarget.ts: fixed the syntax error (matchesAllowEntry"host, entry) → matchesAllowEntry(host, entry)). The new resolveUpstreamTarget / ValidatedUpstreamTarget API is kept.
  • src/routes/proxyRoutes.ts: replaced the import of the non-existent resolveUpstreamAddresses with the real resolveUpstreamTarget, and pinned the validated addresses into the undici agent.
  • src/webhooks/webhook.dispatcher.ts: removed the non-existent isBlockedAddress import and the misuse of resolveUpstreamTarget (it takes a URL, not a bare hostname). Webhook pinning now resolves with dns.lookup, re-checks against BLOCKED_RANGES from webhook.validator, and pins the validated IP through the undici agent.
  • Declared the dependency: added undici@^5.29.0 to package.json dependencies and synced package-lock.json (it was previously only present as a transitive dev dependency of testcontainers).
  • The branch now merges cleanly with main.

Head: e90f5048d5 → f44ce694f7.

Note: I could not run the full TS toolchain here, so CI will confirm the undici type details (dispatcher option and the connect.lookup callback).

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.

Pin resolved upstream addresses to defeat DNS rebinding

2 participants