Conversation
|
@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! 🚀 |
|
Thanks for the contribution! We reviewed this PR while merging the open queue and couldn't merge it yet. Here's what needs fixing:
This branch also has merge conflicts with |
…dresses-to-defeat-dns Resolves merge conflicts against CalloraOrg/Callora-Backend@1518ce6 (68 commit(s) behind) so the PR is mergeable.
|
@greatest0fallt1me thanks for the review — pushed a fix to this branch. What changed:
Head: Note: I could not run the full TS toolchain here, so CI will confirm the undici type details ( |
Overview
This PR closes the DNS rebinding gap in upstream validation. Previously
validateResolvedUpstreamTargetresolved a hostname and checked the resulting addresses, but the subsequentfetchperformed 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 theHostheader.Related Issue
Changes
🔒 Pinned-Address Upstream Validation
[MODIFY]
src/lib/upstreamTarget.tsAgentwhoseconnect/lookuphook returns the already-validated address instead of triggering a fresh DNS resolution.Hostheader, so TLS hostname verification still targets the real host.[MODIFY]
src/routes/proxyRoutes.ts[MODIFY]
src/webhooks/webhook.dispatcher.tsVerification Results
Hostheaderproxy.integration.test.tsSecurity & Failure-Mode Handling
Hostcontinue to use the original hostname, so certificate verification is unaffected.Closes #1261