fix(bitrix): validate ALL resolved IPs for SSRF + stub DNS in tests

- Iterate over all DNS responses instead of just the first IP to prevent
  multi-IP SSRF bypass where a blocked IP appears after a public one
- Make resolver injectable via package-level lookupHost variable
- Replace real-DNS test (google.com) with stubbed resolver covering:
  single public/private IP, DNS failure, empty results, invalid IP,
  multi-IP public→private, private→public, all public, metadata IP,
  IPv6 loopback
- Add fmt import to test file for stubbed error messages
This commit is contained in:
Thành Nguyễn committed 2026-06-11 21:53:47 +07:00
1 parent cf66df44ed
commit 4c500055d2
2 files changed
+68 -22

No files matched your search

+18 -10
View File
@@ -368,6 +368,11 @@ func portalRowToView(row store.BitrixPortalData) bitrixPortalView {
return v
}
// lookupHost is the DNS resolver used by validateSelfHostedDomain.
// Replaced in tests to avoid real network calls and to exercise multi-IP
// SSRF bypass scenarios.
var lookupHost = net.LookupHost
// validateSelfHostedDomain checks a self-hosted Bitrix24 domain for SSRF
// risks and invalid port ranges. Cloud domains (*.bitrix24.*, *.bitrix.info)
// are Bitrix-operated and implicitly trusted — this function is only called
@@ -376,8 +381,7 @@ func portalRowToView(row store.BitrixPortalData) bitrixPortalView {
// Policy:
// - Rejects literal private/loopback/metadata IPs (127.x, 10.x, 192.168.x,
// 169.254.x, ::1, fc00::, etc.)
// - Rejects hostnames that resolve to blocked IPs (defense against
// internal service names like "postgres", "redis", "metadata")
// - Rejects hostnames where ANY resolved IP is blocked (not just the first)
// - Rejects .localhost and .local TLDs (commonly used for local dev)
// - Validates port range 1-65535 when a port is specified
func validateSelfHostedDomain(domain string) error {
@@ -411,20 +415,24 @@ func validateSelfHostedDomain(domain string) error {
return nil
}
// For hostnames, resolve and check the first returned IP.
addrs, err := net.LookupHost(host)
// For hostnames, resolve and check ALL returned IPs. DNS result ordering
// is not a security boundary — a resolver may return a public IP first
// followed by a private one (e.g. split-horizon, fallback addresses).
addrs, err := lookupHost(host)
if err != nil {
return fmt.Errorf("cannot resolve hostname %q", host)
}
if len(addrs) == 0 {
return fmt.Errorf("hostname %q resolved to no addresses", host)
}
ip := net.ParseIP(addrs[0])
if ip == nil {
return fmt.Errorf("resolved address %q is not a valid IP", addrs[0])
}
if security.IsBlocked(ip) {
return fmt.Errorf("hostname %q resolved to blocked IP %s (loopback/private/metadata)", host, ip)
for _, addr := range addrs {
ip := net.ParseIP(addr)
if ip == nil {
return fmt.Errorf("resolved address %q is not a valid IP", addr)
}
if security.IsBlocked(ip) {
return fmt.Errorf("hostname %q resolved to blocked IP %s (loopback/private/metadata)", host, ip)
}
}
return nil
}
+50 -12
View File
@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"strings"
"sync"
"testing"
@@ -694,19 +695,56 @@ func TestValidateSelfHostedDomain_PortRange(t *testing.T) {
}
}
func TestValidateSelfHostedDomain_ValidPublicDomain(t *testing.T) {
// Public domains that resolve to public IPs should pass.
// (Note: this does a real DNS lookup — if the test environment has no
// internet, this will fail. That's acceptable; the SSRF logic is still
// tested via the blocked cases above which use literal IPs.)
valid := []string{
"google.com",
"example.com:443",
func TestValidateSelfHostedDomain_StubbedDNS(t *testing.T) {
// Save and restore the real resolver.
orig := lookupHost
defer func() { lookupHost = orig }()
tests := []struct {
name string
host string
addrs []string
dnsErr error
wantErr bool
}{
{"single public IP", "public.example.com", []string{"8.8.8.8"}, nil, false},
{"public IP with port", "public.example.com", []string{"93.184.216.34"}, nil, false},
{"single private IP", "internal.example.com", []string{"10.0.0.1"}, nil, true},
{"DNS resolution failure", "unresolvable.example.com", nil, fmt.Errorf("no such host"), true},
{"no addresses returned", "empty.example.com", []string{}, nil, true},
{"invalid IP string", "bad.example.com", []string{"not-an-ip"}, nil, true},
// Regression: multi-IP SSRF bypass — public IP first, private IP second.
// DNS ordering is not a security boundary; must reject if ANY IP is blocked.
{"multi-ip public-then-private", "evil.example.com", []string{"8.8.8.8", "10.0.0.1"}, nil, true},
{"multi-ip private-then-public", "evil2.example.com", []string{"192.168.1.1", "8.8.4.4"}, nil, true},
{"multi-ip all public", "safe.example.com", []string{"8.8.8.8", "8.8.4.4"}, nil, false},
{"multi-ip metadata IP", "meta.example.com", []string{"8.8.8.8", "169.254.169.254"}, nil, true},
{"multi-ip IPv6 loopback", "ipv6evil.example.com", []string{"2001:db8::1", "::1"}, nil, true},
}
for _, d := range valid {
if err := validateSelfHostedDomain(d); err != nil {
t.Errorf("should accept %q, got error: %v", d, err)
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
lookupHost = func(host string) ([]string, error) {
if host == tc.host {
return tc.addrs, tc.dnsErr
}
return nil, fmt.Errorf("unexpected host: %s", host)
}
domain := tc.host
// Add port for the "public IP with port" case.
if tc.name == "public IP with port" {
domain = tc.host + ":443"
}
err := validateSelfHostedDomain(domain)
if tc.wantErr && err == nil {
t.Errorf("expected error for %q, got nil", tc.name)
}
if !tc.wantErr && err != nil {
t.Errorf("expected no error for %q, got: %v", tc.name, err)
}
})
}
}