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>
This commit is contained in:
naiba
2026-05-18 15:14:27 +00:00
co-authored by naiba/CloudCode
parent b2294f11dd
commit ea7ad67f03
6 changed files with 370 additions and 169 deletions
+34 -27
View File
@@ -6,6 +6,8 @@ import (
"strings"
"testing"
"time"
"github.com/nezhahq/nezha/pkg/utils"
)
var (
@@ -307,7 +309,7 @@ func TestNotificationTargetRejectsBlockedRanges(t *testing.T) {
for _, rawURL := range cases {
t.Run(rawURL, func(t *testing.T) {
if _, _, err := resolveNotificationTarget(rawURL); err == nil {
if _, _, err := utils.ResolveAllowedHTTPURL(rawURL); err == nil {
t.Fatalf("expected %s to be rejected", rawURL)
}
})
@@ -323,7 +325,7 @@ func TestNotificationTargetAllowsPublicAddresses(t *testing.T) {
for _, rawURL := range cases {
t.Run(rawURL, func(t *testing.T) {
parsedURL, _, err := resolveNotificationTarget(rawURL)
parsedURL, _, err := utils.ResolveAllowedHTTPURL(rawURL)
if err != nil {
t.Fatalf("expected %s to be allowed, got %v", rawURL, err)
}
@@ -334,31 +336,36 @@ func TestNotificationTargetAllowsPublicAddresses(t *testing.T) {
}
}
func TestNotificationHTTPClientPreservesTLSServerName(t *testing.T) {
client, err := newNotificationHTTPClient("https://1.1.1.1/webhook", true)
if err != nil {
t.Fatalf("expected public HTTPS URL to create client: %v", err)
func TestNotificationHTTPClientInvertsVerifyTLSFlag(t *testing.T) {
// newNotificationHTTPClient takes verifyTLS, utils.NewRestrictedHTTPClient
// takes skipVerifyTLS. The wrapper must invert the boolean; if a future
// refactor drops the negation, TLS verification silently turns off.
// SNI / redirect / IP-pinning are covered by pkg/utils/http_test.go.
cases := []struct {
name string
verifyTLS bool
wantSkipVerifyOn bool
}{
{"verifyTLS_true_means_skipVerify_false", true, false},
{"verifyTLS_false_means_skipVerify_true", false, true},
}
transport, ok := client.Transport.(*http.Transport)
if !ok {
t.Fatalf("expected http.Transport, got %T", client.Transport)
}
if transport.TLSClientConfig == nil || transport.TLSClientConfig.ServerName != "1.1.1.1" {
t.Fatalf("expected TLS ServerName 1.1.1.1, got %#v", transport.TLSClientConfig)
}
}
func TestNotificationHTTPClientRejectsRedirects(t *testing.T) {
client, err := newNotificationHTTPClient("https://1.1.1.1/webhook", true)
if err != nil {
t.Fatalf("expected client construction: %v", err)
}
req, err := http.NewRequest(http.MethodGet, "https://1.1.1.1/start", nil)
if err != nil {
t.Fatalf("new request: %v", err)
}
via := []*http.Request{req}
if err := client.CheckRedirect(req, via); err != http.ErrUseLastResponse {
t.Fatalf("expected ErrUseLastResponse, got %v", err)
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
client, err := newNotificationHTTPClient("https://1.1.1.1/webhook", tc.verifyTLS)
if err != nil {
t.Fatalf("expected client construction: %v", err)
}
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")
}
if got := transport.TLSClientConfig.InsecureSkipVerify; got != tc.wantSkipVerifyOn {
t.Fatalf("verifyTLS=%v: expected InsecureSkipVerify=%v, got %v",
tc.verifyTLS, tc.wantSkipVerifyOn, got)
}
})
}
}