From 24d10aced1eb43c5cb23ef40a8e2c2295bbd056e Mon Sep 17 00:00:00 2001 From: simoleo89 <11816867+simoleo89@users.noreply.github.com> Date: Wed, 17 Jun 2026 19:12:01 +0200 Subject: [PATCH] fix(security): harden external-link opening (protocol allow-list + noopener) URLs reached window.open from user/server-controlled content without a protocol check or noopener, allowing reverse-tabnabbing and (for the chat link handler) a javascript:/data: href running in our origin. - add isSafeExternalUrl() (http/https only) + tests; gate the chat link opener (useOnClickChat) and external photo opener with it - SanitizeHtml: afterSanitizeAttributes hook forces rel="noopener noreferrer" on any target=_blank anchor (overrides attacker-supplied rel) - add noopener,noreferrer to the remaining window.open(_blank) sites (YouTube share, external photo, guide forum link); drop a stray console.log --- src/api/utils/SanitizeHtml.test.ts | 16 +++++++ src/api/utils/SanitizeHtml.ts | 15 ++++++- src/api/utils/index.ts | 1 + src/api/utils/isSafeExternalUrl.test.ts | 43 +++++++++++++++++++ src/api/utils/isSafeExternalUrl.ts | 21 +++++++++ src/components/guide-tool/GuideToolView.tsx | 2 +- .../furniture/FurnitureExternalImageView.tsx | 7 ++- src/components/toolbar/YouTubePlayerView.tsx | 2 +- src/hooks/useOnClickChat.ts | 8 +++- 9 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 src/api/utils/isSafeExternalUrl.test.ts create mode 100644 src/api/utils/isSafeExternalUrl.ts diff --git a/src/api/utils/SanitizeHtml.test.ts b/src/api/utils/SanitizeHtml.test.ts index 656d00b..b3c427b 100644 --- a/src/api/utils/SanitizeHtml.test.ts +++ b/src/api/utils/SanitizeHtml.test.ts @@ -86,3 +86,19 @@ describe('SanitizeHtml — preserves the markup the chat/profile UI relies on', expect(parse('a
b').querySelectorAll('br').length).toBe(1); }); }); + +describe('SanitizeHtml — link safety', () => +{ + it('forces rel="noopener noreferrer" on a target=_blank anchor', () => + { + const a = parse('x').querySelector('a'); + expect(a).not.toBeNull(); + expect(a?.getAttribute('rel')).toBe('noopener noreferrer'); + }); + + it('overrides an attacker-supplied rel on a target=_blank anchor', () => + { + const a = parse('x').querySelector('a'); + expect(a?.getAttribute('rel')).toBe('noopener noreferrer'); + }); +}); diff --git a/src/api/utils/SanitizeHtml.ts b/src/api/utils/SanitizeHtml.ts index 39af6e9..9d04c44 100644 --- a/src/api/utils/SanitizeHtml.ts +++ b/src/api/utils/SanitizeHtml.ts @@ -1,10 +1,23 @@ import DOMPurify from 'dompurify'; +// Any link that opens a new browsing context gets a safe rel so it cannot +// reverse-tabnab the opener. Registered once at module load; applies to every +// SanitizeHtml() call (and overrides any attacker-supplied rel). +DOMPurify.addHook('afterSanitizeAttributes', node => +{ + const element = node as Element; + + if((element.tagName === 'A') && element.getAttribute('target')) + { + element.setAttribute('rel', 'noopener noreferrer'); + } +}); + export const SanitizeHtml = (html: string): string => { return DOMPurify.sanitize(html, { ALLOWED_TAGS: [ 'b', 'i', 'u', 'br', 'span', 'div', 'p', 'a', 'strong', 'em', 'img' ], - ALLOWED_ATTR: [ 'href', 'target', 'class', 'style', 'src', 'alt' ], + ALLOWED_ATTR: [ 'href', 'target', 'class', 'style', 'src', 'alt', 'rel' ], ALLOW_DATA_ATTR: false }); }; diff --git a/src/api/utils/index.ts b/src/api/utils/index.ts index 6e19efc..eb64c68 100644 --- a/src/api/utils/index.ts +++ b/src/api/utils/index.ts @@ -15,6 +15,7 @@ export * from './PrefixUtils'; export * from './ProductImageUtility'; export * from './Randomizer'; export * from './RememberLogin'; +export * from './isSafeExternalUrl'; export * from './RoomChatFormatter'; export * from './SanitizeHtml'; export * from './SetLocalStorage'; diff --git a/src/api/utils/isSafeExternalUrl.test.ts b/src/api/utils/isSafeExternalUrl.test.ts new file mode 100644 index 0000000..3e8d5c6 --- /dev/null +++ b/src/api/utils/isSafeExternalUrl.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest'; + +import { isSafeExternalUrl } from './isSafeExternalUrl'; + +/** + * Guard for URLs opened from user-controlled content (chat links, external + * image/photo links). Only plain web URLs may be opened — never script- or + * data-bearing schemes that run in the opener's origin. + */ +describe('isSafeExternalUrl', () => +{ + it('accepts http and https URLs', () => + { + expect(isSafeExternalUrl('http://example.com/path')).toBe(true); + expect(isSafeExternalUrl('https://example.com/path?q=1#x')).toBe(true); + }); + + it('rejects javascript: URLs', () => + { + expect(isSafeExternalUrl('javascript:alert(1)')).toBe(false); + expect(isSafeExternalUrl('JavaScript:alert(1)')).toBe(false); + expect(isSafeExternalUrl(' javascript:alert(1)')).toBe(false); + }); + + it('rejects data: and vbscript: URLs', () => + { + expect(isSafeExternalUrl('data:text/html,')).toBe(false); + expect(isSafeExternalUrl('vbscript:msgbox(1)')).toBe(false); + }); + + it('rejects file: and other non-web schemes', () => + { + expect(isSafeExternalUrl('file:///etc/passwd')).toBe(false); + expect(isSafeExternalUrl('about:blank')).toBe(false); + }); + + it('rejects empty / malformed input', () => + { + expect(isSafeExternalUrl('')).toBe(false); + expect(isSafeExternalUrl(null as unknown as string)).toBe(false); + expect(isSafeExternalUrl('not a url')).toBe(false); + }); +}); diff --git a/src/api/utils/isSafeExternalUrl.ts b/src/api/utils/isSafeExternalUrl.ts new file mode 100644 index 0000000..3e11cd7 --- /dev/null +++ b/src/api/utils/isSafeExternalUrl.ts @@ -0,0 +1,21 @@ +/** + * Returns true only for plain web URLs (http/https). Used to gate URLs that + * originate from user-controlled content before they are opened — never let a + * `javascript:`, `data:`, `vbscript:`, `file:` … scheme reach `window.open`, + * which would run in the opener's origin. + */ +export const isSafeExternalUrl = (url: string): boolean => +{ + if(!url || (typeof url !== 'string')) return false; + + try + { + const protocol = new URL(url.trim()).protocol; + + return ((protocol === 'http:') || (protocol === 'https:')); + } + catch + { + return false; + } +}; diff --git a/src/components/guide-tool/GuideToolView.tsx b/src/components/guide-tool/GuideToolView.tsx index 3fa3bb3..df9acfa 100644 --- a/src/components/guide-tool/GuideToolView.tsx +++ b/src/components/guide-tool/GuideToolView.tsx @@ -321,7 +321,7 @@ export const GuideToolView: FC<{}> = props => return; case 'forum_link': const url: string = GetConfigurationValue('group.homepage.url', '').replace('%groupid%', GetConfigurationValue('guide.help.alpha.groupid', '0')); - window.open(url); + window.open(url, '_blank', 'noopener,noreferrer'); return; } }, [ isHandlingBullyReports, isHandlingGuideRequests, isHandlingHelpRequests, simpleAlert ]); diff --git a/src/components/room/widgets/furniture/FurnitureExternalImageView.tsx b/src/components/room/widgets/furniture/FurnitureExternalImageView.tsx index f55ac1e..14c66af 100644 --- a/src/components/room/widgets/furniture/FurnitureExternalImageView.tsx +++ b/src/components/room/widgets/furniture/FurnitureExternalImageView.tsx @@ -1,6 +1,6 @@ import { FC } from 'react'; import { GetSessionDataManager } from '@nitrots/nitro-renderer'; -import { GetConfigurationValue, LocalizeText, ReportType } from '../../../../api'; +import { GetConfigurationValue, isSafeExternalUrl, LocalizeText, ReportType } from '../../../../api'; import { NitroCardContentView, NitroCardHeaderView, NitroCardView } from '../../../../common'; import { useFurnitureExternalImageWidget, useHelp } from '../../../../hooks'; import { CameraWidgetShowPhotoView } from '../../../camera/views/CameraWidgetShowPhotoView'; @@ -15,10 +15,9 @@ export const FurnitureExternalImageView: FC<{}> = props => const handleOpenFullPhoto = () => { const photoUrl = currentPhotos[currentPhotoIndex].w.replace('_small.png', '.png'); - if (photoUrl) + if (photoUrl && isSafeExternalUrl(photoUrl)) { - console.log('Opened photo URL:', photoUrl); - window.open(photoUrl, '_blank'); + window.open(photoUrl, '_blank', 'noopener,noreferrer'); } }; diff --git a/src/components/toolbar/YouTubePlayerView.tsx b/src/components/toolbar/YouTubePlayerView.tsx index c4c5849..f2f5504 100644 --- a/src/components/toolbar/YouTubePlayerView.tsx +++ b/src/components/toolbar/YouTubePlayerView.tsx @@ -639,7 +639,7 @@ export const YouTubePlayerView: FC<{}> = () => const url = `https://twitter.com/intent/tweet?text=${encodeURIComponent( 'Now watching: https://youtube.com/watch?v=${videoId}', )}`; - window.open(url, '_blank'); + window.open(url, '_blank', 'noopener,noreferrer'); } }} disabled={!videoId} diff --git a/src/hooks/useOnClickChat.ts b/src/hooks/useOnClickChat.ts index 96e2ae1..67589e1 100644 --- a/src/hooks/useOnClickChat.ts +++ b/src/hooks/useOnClickChat.ts @@ -1,5 +1,5 @@ import { useBetween } from 'use-between'; -import { LocalizeText } from '../api'; +import { isSafeExternalUrl, LocalizeText } from '../api'; import { useNotification } from './notification'; const useOnClickChatState = () => @@ -15,9 +15,13 @@ const useOnClickChatState = () => const url = event.target.href; + // Never open a URL that came from chat unless it is a plain web link — + // a javascript:/data: href would otherwise run in our origin. + if(!isSafeExternalUrl(url)) return; + showConfirm(LocalizeText('chat.confirm.openurl', [ 'url' ], [ url ]), () => { - window.open(url, '_blank'); + window.open(url, '_blank', 'noopener,noreferrer'); }, null, null, null, LocalizeText('generic.alert.title'), null); };