diff --git a/docs/releases.md b/docs/releases.md index de6f9a78..cb9a6958 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1884,5 +1884,6 @@ This is the first iOS release in 3 years, focusing on stability fixes as per the **Bug fixes + maintenance:** +* Tighten web push endpoint allow-list regex to prevent SSRF via unanchored pattern matching ([GHSA-w9hq-5jg7-q4j7](https://github.com/binwiederhier/ntfy/security/advisories/GHSA-w9hq-5jg7-q4j7), thanks to [@MightyNawaf](https://github.com/MightyNawaf) for reporting) * Fix web app not allowing access tokens to be changed to never expire ([#1693](https://github.com/binwiederhier/ntfy/issues/1693)/[#1694](https://github.com/binwiederhier/ntfy/pull/1694), thanks to [@lastsamurai26](https://github.com/lastsamurai26) for reporting and to [@ShipItAndPray](https://github.com/ShipItAndPray) for fixing) * Fix web app crashing on account page for tokens without a last access time ([#1651](https://github.com/binwiederhier/ntfy/issues/1651), [#1684](https://github.com/binwiederhier/ntfy/issues/1684), thanks to [@Pulsar7](https://github.com/Pulsar7) and [@rzhli](https://github.com/rzhli) for reporting) diff --git a/server/server_webpush.go b/server/server_webpush.go index f98b8e91..70f34669 100644 --- a/server/server_webpush.go +++ b/server/server_webpush.go @@ -7,7 +7,6 @@ import ( "fmt" "net/http" "regexp" - "strings" "github.com/SherClockHolmes/webpush-go" "heckel.io/ntfy/v2/log" @@ -24,32 +23,35 @@ const ( webPushTopicSubscribeLimit = 50 ) -var ( - webPushAllowedEndpointsPatterns = []string{ - "https://*.google.com/", - "https://*.googleapis.com/", - "https://*.mozilla.com/", - "https://*.mozaws.net/", - "https://*.windows.com/", - "https://*.microsoft.com/", - "https://*.apple.com/", - } - webPushAllowedEndpointsRegex *regexp.Regexp -) +// webPushAllowedEndpointsRegexes is the host-level allow-list of web push services ntfy +// will deliver to. Each regex anchors the scheme and matches the stable service host, +// followed by the authority/path boundary "/". Instance-specific labels (e.g. the +// "wns2-" prefix on Windows Notification Service hosts) are wildcarded with +// a single-label pattern ([^/]+) that cannot span into the path. +// See GHSA-w9hq-5jg7-q4j7 for why wildcarding the entire host is insufficient. +var webPushAllowedEndpointsRegexes = []*regexp.Regexp{ + regexp.MustCompile(`^https://fcm\.googleapis\.com/`), + regexp.MustCompile(`^https://jmt17\.google\.com/`), + regexp.MustCompile(`^https://updates\.push\.services\.mozilla\.com/`), + regexp.MustCompile(`^https://[^/]+\.mozaws\.net/`), + regexp.MustCompile(`^https://web\.push\.apple\.com/`), + regexp.MustCompile(`^https://[^/]+\.notify\.windows\.com/`), +} -func init() { - for i, pattern := range webPushAllowedEndpointsPatterns { - webPushAllowedEndpointsPatterns[i] = strings.ReplaceAll(strings.ReplaceAll(pattern, ".", "\\."), "*", ".+") +func webPushEndpointAllowed(endpoint string) bool { + for _, re := range webPushAllowedEndpointsRegexes { + if re.MatchString(endpoint) { + return true + } } - allPatterns := fmt.Sprintf("^(%s)", strings.Join(webPushAllowedEndpointsPatterns, "|")) - webPushAllowedEndpointsRegex = regexp.MustCompile(allPatterns) + return false } func (s *Server) handleWebPushUpdate(w http.ResponseWriter, r *http.Request, v *visitor) error { req, err := readJSONWithLimit[apiWebPushUpdateSubscriptionRequest](r.Body, jsonBodyBytesLimit, false) if err != nil || req.Endpoint == "" || req.P256dh == "" || req.Auth == "" { return errHTTPBadRequestWebPushSubscriptionInvalid - } else if !webPushAllowedEndpointsRegex.MatchString(req.Endpoint) { + } else if !webPushEndpointAllowed(req.Endpoint) { return errHTTPBadRequestWebPushEndpointUnknown } else if len(req.Topics) > webPushTopicSubscribeLimit { return errHTTPBadRequestWebPushTopicCountTooHigh diff --git a/server/server_webpush_test.go b/server/server_webpush_test.go index bba13db4..e69da16d 100644 --- a/server/server_webpush_test.go +++ b/server/server_webpush_test.go @@ -87,6 +87,78 @@ func TestServer_WebPush_TopicAdd_InvalidEndpoint(t *testing.T) { }) } +func TestServer_WebPush_EndpointRegex(t *testing.T) { + // Synthetic endpoint samples representing each supported push service host shape. + allowed := []string{ + // Google FCM (legacy send, webpush, preprod webpush) + "https://fcm.googleapis.com/fcm/send/FAKETOKEN:APA91b-placeholder-not-a-real-token", + "https://fcm.googleapis.com/wp/FAKETOKEN:APA91b-placeholder-not-a-real-token", + "https://fcm.googleapis.com/preprod/wp/FAKETOKEN:APA91b-placeholder-not-a-real-token", + "https://jmt17.google.com/fcm/send/FAKETOKEN:APA91b-placeholder-not-a-real-token", + // Mozilla autopush (v1 legacy, v2 current, plus AWS-hosted infra) + "https://updates.push.services.mozilla.com/wpush/v1/placeholder-not-a-real-token", + "https://updates.push.services.mozilla.com/wpush/v2/placeholder-not-a-real-token", + "https://autopush.mozaws.net/wpush/v1/placeholder-not-a-real-token", + // Apple Web Push + "https://web.push.apple.com/placeholder-not-a-real-token", + // Microsoft WNS: instance-specific "wns2-" prefix is wildcarded + "https://wns2-bn3p.notify.windows.com/w/?token=placeholder", + "https://wns2-ch1p.notify.windows.com/w/?token=placeholder", + "https://wns2-par02p.notify.windows.com/w/?token=placeholder", + "https://wns2-pn1p.notify.windows.com/w/?token=placeholder", + "https://wns2-am3p.notify.windows.com/w/?token=placeholder", + } + denied := []string{ + // HTTP (not HTTPS) + "http://fcm.googleapis.com/fcm/send/abc", + // Unrelated host + "https://attacker.example.com/webpush", + // GHSA-w9hq-5jg7-q4j7 bypass: allowed host embedded in path + "https://attacker.com/x.google.com/push", + "https://attacker.example.com/fcm.googleapis.com/fcm/send/abc", + "https://evil.test/web.push.apple.com/3/device/abc", + "https://ntfytest.requestcatcher.com/path.google.com/push", + "https://ntfytest.requestcatcher.com/a.google.com/toto", + "https://ntfytest.requestcatcher.com/bypass.google.com/test", + "https://webhook.site/86e94e2e-2af4-4a31-a80b-e2f335cc6495/path.google.com/push", + "https://webhook.site/86e94e2e-2af4-4a31-a80b-e2f335cc6495/bypass.google.com/", + // Allowed host as a prefix of a different host (no separating slash) + "https://fcm.googleapis.com.attacker.com/fcm/send/abc", + "https://web.push.apple.com.evil.test/tok", + // Allowed host as a suffix of a different host (no separating dot) + "https://evilgoogle.com/", + "https://notapple.com/", + // Credentials/userinfo in the URL pointing at a different host + "https://fcm.googleapis.com@attacker.com/fcm/send/abc", + // Previously allowed by the wildcard allowlist but not actually used by Web Push + "https://api.push.apple.com/3/device/abc", + "https://android.googleapis.com/send/xyz", + "https://login.microsoft.com/anything", + // Bare notify.windows.com with no subdomain label + "https://notify.windows.com/w/?token=abc", + } + for _, endpoint := range allowed { + require.Truef(t, webPushEndpointAllowed(endpoint), "expected endpoint to be allowed: %s", endpoint) + } + for _, endpoint := range denied { + require.Falsef(t, webPushEndpointAllowed(endpoint), "expected endpoint to be denied: %s", endpoint) + } +} + +func TestServer_WebPush_TopicAdd_BypassAttempt(t *testing.T) { + // Regression test for GHSA-w9hq-5jg7-q4j7: the allow-list regex previously had no + // end anchor, so a URL like https://attacker.example.com/x.google.com/... passed + // validation and caused the server to deliver push payloads to attacker-controlled + // endpoints (SSRF + message exfiltration via attacker-supplied p256dh key). + forEachBackend(t, func(t *testing.T, databaseURL string) { + s := newTestServer(t, newTestConfigWithWebPush(t, databaseURL)) + + response := request(t, s, "POST", "/v1/webpush", payloadForTopics(t, []string{"test-topic"}, "https://attacker.example.com/x.google.com/push"), nil) + require.Equal(t, 400, response.Code) + require.Equal(t, `{"code":40039,"http":400,"error":"invalid request: web push endpoint unknown"}`+"\n", response.Body.String()) + }) +} + func TestServer_WebPush_TopicAdd_TooManyTopics(t *testing.T) { forEachBackend(t, func(t *testing.T, databaseURL string) { s := newTestServer(t, newTestConfigWithWebPush(t, databaseURL))