mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
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
This commit is contained in:
parent
c84642ac16
commit
4e56595866
3 changed files with 74 additions and 1 deletions
|
|
@ -91,6 +91,13 @@ export class Adapter<T extends CacheStorage = CacheStorage> {
|
|||
* 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);
|
||||
|
|
|
|||
|
|
@ -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':
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue