From 4e5659586605cedc7e1920e81d2b26d208bfea3a Mon Sep 17 00:00:00 2001 From: Biswa Date: Sun, 3 May 2026 16:00:08 +0530 Subject: [PATCH] fix(service-worker): prevent open redirect via protocol-relative URLs This fix addresses an open redirect vulnerability in the Service Worker where protocol-relative URLs (e.g., //evil.com) could be used to redirect users to attacker-controlled domains via push notifications. Changes: - driver.ts: Add validation to reject protocol-relative URLs (//) and backslash-prefixed URLs (\) in notification action URLs - driver.ts: Use URL.canParse() to allow explicit absolute URLs while properly handling relative URLs - adapter.ts: Add validation to reject protocol-relative URLs in parseUrl - Add unit tests for security validation in notification click handling Fixes: Open redirect vulnerability in push notification handling --- packages/service-worker/worker/src/adapter.ts | 7 +++ packages/service-worker/worker/src/driver.ts | 24 +++++++++- .../service-worker/worker/test/happy_spec.ts | 44 +++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) diff --git a/packages/service-worker/worker/src/adapter.ts b/packages/service-worker/worker/src/adapter.ts index 63505b1691c..4a3929184e1 100644 --- a/packages/service-worker/worker/src/adapter.ts +++ b/packages/service-worker/worker/src/adapter.ts @@ -91,6 +91,13 @@ export class Adapter { * Parse a URL into its different parts, such as `origin`, `path` and `search`. */ parseUrl(url: string, relativeTo?: string): {origin: string; path: string; search: string} { + // Reject protocol-relative URLs to prevent URL injection attacks. + // Protocol-relative URLs (e.g., //evil.com) can resolve to external domains + // when the URL constructor processes them. + if (url.startsWith('//') || url.startsWith('\\')) { + throw new Error('Protocol-relative URLs are not allowed in Service Worker'); + } + // Workaround a Safari bug, see // https://github.com/angular/angular/issues/31061#issuecomment-503637978 const parsed = !relativeTo ? new URL(url) : new URL(url, relativeTo); diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index c48a63c230a..08cc7bdd209 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -437,7 +437,29 @@ export class Driver implements Debuggable, UpdateSource { const onActionClick = notification?.data?.onActionClick?.[notificationAction]; - const urlToOpen = new URL(onActionClick?.url ?? '', this.scope.registration.scope).href; + const actionUrl = onActionClick?.url ?? ''; + + // Validate URL to prevent open redirect attacks via protocol-relative URLs. + // Protocol-relative URLs (e.g., //evil.com) can escape the service worker scope + // and redirect users to attacker-controlled domains. + if (actionUrl.startsWith('//') || actionUrl.startsWith('\\')) { + this.debugger.log(`Rejecting potentially malicious URL in notification action: ${actionUrl}`); + return; + } + + let urlToOpen: string; + + // Handle absolute URLs (e.g., http://example.com/path) differently from relative URLs. + // Absolute URLs are explicitly set by developers and should be allowed. + if (URL.canParse(actionUrl)) { + urlToOpen = new URL(actionUrl).href; + } else { + // For relative URLs, resolve against scope. + // Relative URLs that stay on the same origin are allowed. + // Only protocol-relative URLs (which we already rejected above) can change the origin. + const baseUrl = new URL(this.scope.registration.scope); + urlToOpen = new URL(actionUrl, baseUrl).href; + } switch (onActionClick?.operation) { case 'openWindow': diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index 33ba18ebd1b..0dfe6da6edc 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -811,6 +811,50 @@ import {envIsSupported} from '../testing/utils'; ); expect(scope.clients.openWindow).toHaveBeenCalledWith(`${scope.registration.scope}`); }); + + it('should not open window for protocol-relative URLs (security)', async () => { + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + + spyOn(scope.clients, 'openWindow'); + + await driver.initialized; + await scope.handleClick( + { + title: 'Malicious notification', + body: 'Test body with malicious url', + data: { + onActionClick: { + default: {operation: 'openWindow', url: '//attacker.com/phishing'}, + }, + }, + }, + 'default', + ); + // Should NOT call openWindow with malicious URL + expect(scope.clients.openWindow).not.toHaveBeenCalled(); + }); + + it('should not open window for backslash-prefixed URLs (security)', async () => { + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + + spyOn(scope.clients, 'openWindow'); + + await driver.initialized; + await scope.handleClick( + { + title: 'Malicious notification', + body: 'Test body with malicious url', + data: { + onActionClick: { + default: {operation: 'openWindow', url: '\\attacker.com/phishing'}, + }, + }, + }, + 'default', + ); + // Should NOT call openWindow with malicious URL + expect(scope.clients.openWindow).not.toHaveBeenCalled(); + }); }); describe('`focusLastFocusedOrOpen` operation', () => {