Tighten web push endpoint allow list
This commit is contained in:
parent
1956c88392
commit
fa83d68754
3 changed files with 94 additions and 19 deletions
|
|
@ -1884,5 +1884,6 @@ This is the first iOS release in 3 years, focusing on stability fixes as per the
|
||||||
|
|
||||||
**Bug fixes + maintenance:**
|
**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 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)
|
* 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)
|
||||||
|
|
|
||||||
|
|
@ -7,7 +7,6 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
|
||||||
|
|
||||||
"github.com/SherClockHolmes/webpush-go"
|
"github.com/SherClockHolmes/webpush-go"
|
||||||
"heckel.io/ntfy/v2/log"
|
"heckel.io/ntfy/v2/log"
|
||||||
|
|
@ -24,32 +23,35 @@ const (
|
||||||
webPushTopicSubscribeLimit = 50
|
webPushTopicSubscribeLimit = 50
|
||||||
)
|
)
|
||||||
|
|
||||||
var (
|
// webPushAllowedEndpointsRegexes is the host-level allow-list of web push services ntfy
|
||||||
webPushAllowedEndpointsPatterns = []string{
|
// will deliver to. Each regex anchors the scheme and matches the stable service host,
|
||||||
"https://*.google.com/",
|
// followed by the authority/path boundary "/". Instance-specific labels (e.g. the
|
||||||
"https://*.googleapis.com/",
|
// "wns2-<region>" prefix on Windows Notification Service hosts) are wildcarded with
|
||||||
"https://*.mozilla.com/",
|
// a single-label pattern ([^/]+) that cannot span into the path.
|
||||||
"https://*.mozaws.net/",
|
// See GHSA-w9hq-5jg7-q4j7 for why wildcarding the entire host is insufficient.
|
||||||
"https://*.windows.com/",
|
var webPushAllowedEndpointsRegexes = []*regexp.Regexp{
|
||||||
"https://*.microsoft.com/",
|
regexp.MustCompile(`^https://fcm\.googleapis\.com/`),
|
||||||
"https://*.apple.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/`),
|
||||||
}
|
}
|
||||||
webPushAllowedEndpointsRegex *regexp.Regexp
|
|
||||||
)
|
|
||||||
|
|
||||||
func init() {
|
func webPushEndpointAllowed(endpoint string) bool {
|
||||||
for i, pattern := range webPushAllowedEndpointsPatterns {
|
for _, re := range webPushAllowedEndpointsRegexes {
|
||||||
webPushAllowedEndpointsPatterns[i] = strings.ReplaceAll(strings.ReplaceAll(pattern, ".", "\\."), "*", ".+")
|
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 {
|
func (s *Server) handleWebPushUpdate(w http.ResponseWriter, r *http.Request, v *visitor) error {
|
||||||
req, err := readJSONWithLimit[apiWebPushUpdateSubscriptionRequest](r.Body, jsonBodyBytesLimit, false)
|
req, err := readJSONWithLimit[apiWebPushUpdateSubscriptionRequest](r.Body, jsonBodyBytesLimit, false)
|
||||||
if err != nil || req.Endpoint == "" || req.P256dh == "" || req.Auth == "" {
|
if err != nil || req.Endpoint == "" || req.P256dh == "" || req.Auth == "" {
|
||||||
return errHTTPBadRequestWebPushSubscriptionInvalid
|
return errHTTPBadRequestWebPushSubscriptionInvalid
|
||||||
} else if !webPushAllowedEndpointsRegex.MatchString(req.Endpoint) {
|
} else if !webPushEndpointAllowed(req.Endpoint) {
|
||||||
return errHTTPBadRequestWebPushEndpointUnknown
|
return errHTTPBadRequestWebPushEndpointUnknown
|
||||||
} else if len(req.Topics) > webPushTopicSubscribeLimit {
|
} else if len(req.Topics) > webPushTopicSubscribeLimit {
|
||||||
return errHTTPBadRequestWebPushTopicCountTooHigh
|
return errHTTPBadRequestWebPushTopicCountTooHigh
|
||||||
|
|
|
||||||
|
|
@ -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-<region>" 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) {
|
func TestServer_WebPush_TopicAdd_TooManyTopics(t *testing.T) {
|
||||||
forEachBackend(t, func(t *testing.T, databaseURL string) {
|
forEachBackend(t, func(t *testing.T, databaseURL string) {
|
||||||
s := newTestServer(t, newTestConfigWithWebPush(t, databaseURL))
|
s := newTestServer(t, newTestConfigWithWebPush(t, databaseURL))
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue