fix(ddns): apply SSRF defense to webhook provider

GHSA-6x26-5727-rrm9: a low-privilege member could point a DDNS webhook at internal or loopback hosts and the dashboard would dial them with the unrestricted utils.HttpClient. Extract the notification SSRF defenses (CIDR blocklist, IP-pin DialContext, SNI preservation, redirect rejection) into reusable helpers in pkg/utils (NewRestrictedHTTPClient / ResolveAllowedHTTPURL / buildRestrictedHTTPClient) and route the DDNS webhook through the same path. Replace the notification inline implementation with a thin wrapper to keep behaviour identical. Side improvements collected by the refactor: - prepareRequest now resolves DNS once and returns the paired client, so the dialer's pinned IP and the validated URL stay in sync (no more double resolution between prepareRequest and SetRecords). - response body is drained and closed. - HttpClient / HttpClientSkipTlsVerify are explicitly tagged unsafe for attacker-controlled URLs. Tests cover: hermetic SNI preservation, redirect rejection, dial pin to the vetted IP, the full blocked-CIDR list at the webhook entry point, and the verifyTLS↔skipVerifyTLS inversion in the notification wrapper. Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
2026-09-19 17:50:12 +00:00 · 2026-05-18 15:14:27 +00:00
parent 05e5da2535
commit e7c2e453c0
6 changed files with 370 additions and 169 deletions
@@ -1,16 +1,53 @@
 package utils

 import (
+	"context"
 	"crypto/tls"
+	"errors"
+	"net"
 	"net/http"
+	"net/netip"
+	"net/url"
 	"time"
 )

+// HttpClient / HttpClientSkipTlsVerify must not be used to dispatch
+// requests to user-controlled URLs (SSRF risk, GHSA-6x26-5727-rrm9).
+// For any attacker-controlled URL use NewRestrictedHTTPClient instead.
 var (
 	HttpClientSkipTlsVerify *http.Client
 	HttpClient              *http.Client
 )

+var ErrHTTPURLTargetNotAllowed = errors.New("HTTP URL target is not allowed")
+
+var blockedHTTPClientCIDRs = mustParseHTTPClientCIDRs([]string{
+	"0.0.0.0/8",
+	"10.0.0.0/8",
+	"100.64.0.0/10",
+	"127.0.0.0/8",
+	"169.254.0.0/16",
+	"172.16.0.0/12",
+	"192.0.0.0/24",
+	"192.0.2.0/24",
+	"192.168.0.0/16",
+	"198.18.0.0/15",
+	"198.51.100.0/24",
+	"203.0.113.0/24",
+	"224.0.0.0/4",
+	"240.0.0.0/4",
+	"::/128",
+	"::1/128",
+	"::ffff:0:0/96",
+	"64:ff9b::/96",
+	"100::/64",
+	"2001::/23",
+	"2001:db8::/32",
+	"fc00::/7",
+	"fe80::/10",
+	"ff00::/8",
+})
+
 func init() {
 	HttpClientSkipTlsVerify = httpClient(_httpClient{
 		Transport: httpTransport(_httpTransport{
@@ -47,3 +84,106 @@ func httpClient(conf _httpClient) *http.Client {
 		Timeout:   time.Minute * 10,
 	}
 }
+
+func NewRestrictedHTTPClient(rawURL string, skipVerifyTLS bool) (*http.Client, error) {
+	parsedURL, ip, err := ResolveAllowedHTTPURL(rawURL)
+	if err != nil {
+		return nil, err
+	}
+	return buildRestrictedHTTPClient(parsedURL, ip, skipVerifyTLS), nil
+}
+
+// buildRestrictedHTTPClient assembles a client whose DialContext is pinned to
+// the already-vetted IP. Separated from NewRestrictedHTTPClient so tests can
+// exercise the SNI / redirect behavior without relying on live DNS.
+func buildRestrictedHTTPClient(parsedURL *url.URL, ip net.IP, skipVerifyTLS bool) *http.Client {
+	port := parsedURL.Port()
+	if port == "" {
+		if parsedURL.Scheme == "https" {
+			port = "443"
+		} else {
+			port = "80"
+		}
+	}
+	// Pin outbound webhooks to the vetted IP so DNS changes cannot retarget private hosts.
+	targetAddress := net.JoinHostPort(ip.String(), port)
+	dialer := &net.Dialer{}
+
+	return &http.Client{
+		Transport: &http.Transport{
+			DialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
+				return dialer.DialContext(ctx, network, targetAddress)
+			},
+			TLSClientConfig: &tls.Config{InsecureSkipVerify: skipVerifyTLS, ServerName: parsedURL.Hostname()},
+		},
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			return http.ErrUseLastResponse
+		},
+		Timeout: time.Minute * 10,
+	}
+}
+
+func ResolveAllowedHTTPURL(rawURL string) (*url.URL, net.IP, error) {
+	parsedURL, err := url.Parse(rawURL)
+	if err != nil {
+		return nil, nil, err
+	}
+	if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
+		return nil, nil, ErrHTTPURLTargetNotAllowed
+	}
+
+	host := parsedURL.Hostname()
+	if host == "" {
+		return nil, nil, ErrHTTPURLTargetNotAllowed
+	}
+	if ip := net.ParseIP(host); ip != nil {
+		if !HTTPURLTargetIPAllowed(ip) {
+			return nil, nil, ErrHTTPURLTargetNotAllowed
+		}
+		return parsedURL, ip, nil
+	}
+
+	ips, err := net.LookupIP(host)
+	if err != nil {
+		return nil, nil, err
+	}
+	if len(ips) == 0 {
+		return nil, nil, ErrHTTPURLTargetNotAllowed
+	}
+	for _, ip := range ips {
+		if !HTTPURLTargetIPAllowed(ip) {
+			return nil, nil, ErrHTTPURLTargetNotAllowed
+		}
+	}
+
+	return parsedURL, ips[0], nil
+}
+
+func HTTPURLTargetIPAllowed(ip net.IP) bool {
+	parsedIP, ok := netipFromIP(ip)
+	if !ok {
+		return false
+	}
+	for _, cidr := range blockedHTTPClientCIDRs {
+		if cidr.Contains(parsedIP) {
+			return false
+		}
+	}
+	return parsedIP.IsGlobalUnicast()
+}
+
+func netipFromIP(ip net.IP) (netip.Addr, bool) {
+	parsedIP, ok := netip.AddrFromSlice(ip)
+	if !ok {
+		return netip.Addr{}, false
+	}
+	return parsedIP.Unmap(), true
+}
+
+func mustParseHTTPClientCIDRs(cidrs []string) []netip.Prefix {
+	prefixes := make([]netip.Prefix, 0, len(cidrs))
+	for _, cidr := range cidrs {
+		prefixes = append(prefixes, netip.MustParsePrefix(cidr))
+	}
+	return prefixes
+}
@@ -0,0 +1,110 @@
+package utils
+
+import (
+	"net"
+	"net/http"
+	"net/url"
+	"testing"
+	"time"
+)
+
+func TestBuildRestrictedHTTPClientPreservesHostnameAsTLSServerName(t *testing.T) {
+	// Construct a hostname URL paired with an arbitrary public IP so we exercise
+	// the SNI preservation path without depending on live DNS in unit tests.
+	parsed, err := url.Parse("https://example.com/webhook")
+	if err != nil {
+		t.Fatalf("parse url: %v", err)
+	}
+	pinnedIP := net.ParseIP("1.1.1.1")
+	if pinnedIP == nil {
+		t.Fatalf("expected valid pinned IP")
+	}
+
+	client := buildRestrictedHTTPClient(parsed, pinnedIP, false)
+	transport, ok := client.Transport.(*http.Transport)
+	if !ok {
+		t.Fatalf("expected *http.Transport, got %T", client.Transport)
+	}
+	if transport.TLSClientConfig == nil {
+		t.Fatalf("expected TLSClientConfig to be set")
+	}
+	// SNI must come from the original URL hostname so the certificate validates
+	// the intended hostname, not the pinned dial IP.
+	if got := transport.TLSClientConfig.ServerName; got != "example.com" {
+		t.Fatalf("expected ServerName example.com, got %q", got)
+	}
+	if transport.TLSClientConfig.ServerName == pinnedIP.String() {
+		t.Fatalf("ServerName must not be the pinned IP, got %q", transport.TLSClientConfig.ServerName)
+	}
+	if transport.TLSClientConfig.InsecureSkipVerify {
+		t.Fatalf("expected verifyTLS path (InsecureSkipVerify=false)")
+	}
+}
+
+func TestBuildRestrictedHTTPClientHonorsSkipVerifyTLS(t *testing.T) {
+	parsed, _ := url.Parse("https://example.com/webhook")
+	client := buildRestrictedHTTPClient(parsed, net.ParseIP("1.1.1.1"), true)
+	transport := client.Transport.(*http.Transport)
+	if !transport.TLSClientConfig.InsecureSkipVerify {
+		t.Fatalf("expected InsecureSkipVerify=true when skipVerifyTLS=true")
+	}
+}
+
+func TestBuildRestrictedHTTPClientRejectsRedirects(t *testing.T) {
+	parsed, _ := url.Parse("https://example.com/start")
+	client := buildRestrictedHTTPClient(parsed, net.ParseIP("1.1.1.1"), false)
+	req, err := http.NewRequest(http.MethodGet, "https://example.com/start", nil)
+	if err != nil {
+		t.Fatalf("new request: %v", err)
+	}
+	if err := client.CheckRedirect(req, []*http.Request{req}); err != http.ErrUseLastResponse {
+		t.Fatalf("expected ErrUseLastResponse, got %v", err)
+	}
+}
+
+// TestBuildRestrictedHTTPClientPinsDialToVettedIP confirms DialContext routes
+// to the pinned IP even when the request URL uses a different hostname,
+// preventing DNS rebinding from retargeting traffic.
+func TestBuildRestrictedHTTPClientPinsDialToVettedIP(t *testing.T) {
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	defer listener.Close()
+	_, port, err := net.SplitHostPort(listener.Addr().String())
+	if err != nil {
+		t.Fatalf("split host port: %v", err)
+	}
+
+	accepted := make(chan string, 1)
+	go func() {
+		conn, err := listener.Accept()
+		if err != nil {
+			accepted <- ""
+			return
+		}
+		accepted <- conn.LocalAddr().String()
+		conn.Close()
+	}()
+
+	requestURL := "http://example.com:" + port + "/"
+	parsed, _ := url.Parse(requestURL)
+	pinned := net.ParseIP("127.0.0.1")
+	client := buildRestrictedHTTPClient(parsed, pinned, false)
+	client.Timeout = 2 * time.Second
+
+	req, _ := http.NewRequest(http.MethodGet, requestURL, nil)
+	resp, _ := client.Do(req)
+	if resp != nil {
+		resp.Body.Close()
+	}
+
+	select {
+	case addr := <-accepted:
+		if addr == "" {
+			t.Fatalf("listener accept failed")
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatalf("expected dial to reach pinned IP 127.0.0.1:%s, listener did not accept", port)
+	}
+}