diff --git a/src/api/utils/RoomChatFormatter.test.ts b/src/api/utils/RoomChatFormatter.test.ts new file mode 100644 index 0000000..403fd87 --- /dev/null +++ b/src/api/utils/RoomChatFormatter.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from 'vitest'; + +import { RoomChatFormatter } from './RoomChatFormatter'; + +/** + * Security + behaviour suite for the chat formatter. + * + * The formatter output is injected into the DOM via `dangerouslySetInnerHTML` + * in ChatWidgetMessageView, so the security contract is: after the browser + * parses the formatted string as HTML, NO attacker-controlled executable + * markup may survive (no '); + expect(div.querySelector('script')).toBeNull(); + }); + + it('does not produce an element with an onerror handler', () => + { + const div = parse(''); + const img = div.querySelector('img'); + expect(img).toBeNull(); + }); + + it('does not produce an element with an onload handler', () => + { + const div = parse(''); + expect(div.querySelector('svg')).toBeNull(); + }); + + it('does not keep a javascript: href on an anchor', () => + { + const div = parse('x'); + expect(div.querySelector('a[href^="javascript:"]')).toBeNull(); + }); + + it('strips event-handler attributes injected via a font tag', () => + { + const div = parse('hi'); + expect(div.querySelector('[onload]')).toBeNull(); + expect(div.innerHTML.toLowerCase()).not.toContain('onload'); + }); + + it('neutralises numeric-entity-encoded image injection (<img …>)', () => + { + const div = parse('<img src=x onerror=alert(1)>'); + expect(div.querySelector('img')).toBeNull(); + }); + + it('neutralises hex-entity-encoded image injection (<img …)', () => + { + const div = parse('<img src=x onerror=alert(1)>'); + expect(div.querySelector('img')).toBeNull(); + }); + + it('does not allow an arbitrary style/background via font color', () => + { + const div = parse('hi'); + expect(div.innerHTML.toLowerCase()).not.toContain('javascript:'); + }); +}); + +describe('RoomChatFormatter — legitimate markup is preserved', () => +{ + it('renders [b]…[/b] as ', () => + { + const div = parse('[b]hello[/b]'); + const strong = div.querySelector('strong'); + expect(strong).not.toBeNull(); + expect(strong?.textContent).toBe('hello'); + }); + + it('renders a whitelisted font colour as a coloured span', () => + { + const div = parse('hi'); + const span = div.querySelector('span'); + expect(span).not.toBeNull(); + expect((span as HTMLElement).style.color).toBe('red'); + expect(span?.textContent).toBe('hi'); + }); + + it('drops a non-whitelisted font colour but keeps the inner text', () => + { + const div = parse('hi'); + expect(div.textContent).toContain('hi'); + expect(div.innerHTML.toLowerCase()).not.toContain('notacolour'); + }); + + it('passes plain text through unchanged', () => + { + const div = parse('just a normal message'); + expect(div.textContent).toBe('just a normal message'); + }); + + it('converts newlines to
', () => + { + const div = parse('line1\nline2'); + expect(div.querySelectorAll('br').length).toBe(1); + }); +}); diff --git a/src/api/utils/SanitizeHtml.test.ts b/src/api/utils/SanitizeHtml.test.ts new file mode 100644 index 0000000..b3c427b --- /dev/null +++ b/src/api/utils/SanitizeHtml.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from 'vitest'; + +import { SanitizeHtml } from './SanitizeHtml'; + +/** + * SanitizeHtml is the project's shared HTML sanitiser (DOMPurify with a fixed + * allow-list). It is the load-bearing defence wherever user/server-controlled + * strings are rendered via `dangerouslySetInnerHTML`. These tests pin both the + * security guarantee (no executable markup survives) and the formatting + * guarantee (the limited markup the chat/profile UI relies on is preserved), + * using jsdom's real parser as the oracle. + */ +const parse = (html: string): HTMLDivElement => +{ + const div = document.createElement('div'); + div.innerHTML = SanitizeHtml(html); + return div; +}; + +describe('SanitizeHtml — neutralises dangerous markup', () => +{ + it('removes ').querySelector('script')).toBeNull(); + }); + + it('strips inline event handlers (onerror) from allowed tags', () => + { + const div = parse(''); + const img = div.querySelector('img'); + // img tag itself is allow-listed, but the handler must be gone + expect(img?.getAttribute('onerror')).toBeNull(); + expect(div.innerHTML.toLowerCase()).not.toContain('onerror'); + }); + + it('drops javascript: URLs on anchors', () => + { + const div = parse('x'); + expect(div.querySelector('a[href^="javascript:"]')).toBeNull(); + }); + + it('removes and its onload handler', () => + { + const div = parse(''); + expect(div.innerHTML.toLowerCase()).not.toContain('onload'); + }); + + it('removes //', () => + { + const div = parse('abc'); + expect(div.querySelector('b')).not.toBeNull(); + expect(div.querySelector('i')).not.toBeNull(); + expect(div.querySelector('u')).not.toBeNull(); + }); + + it('keeps / (RoomChatFormatter output)', () => + { + const div = parse('xy'); + expect(div.querySelector('strong')).not.toBeNull(); + expect(div.querySelector('em')).not.toBeNull(); + }); + + it('keeps a coloured span (RoomChatFormatter output)', () => + { + const div = parse('hi'); + const span = div.querySelector('span'); + expect(span).not.toBeNull(); + expect((span as HTMLElement).style.color).toBe('red'); + }); + + it('keeps
', () => + { + 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/chat-history/ChatHistoryView.tsx b/src/components/chat-history/ChatHistoryView.tsx index 0a1283e..c0f5d5b 100644 --- a/src/components/chat-history/ChatHistoryView.tsx +++ b/src/components/chat-history/ChatHistoryView.tsx @@ -1,6 +1,6 @@ import { AddLinkEventTracker, ILinkEventTracker, RemoveLinkEventTracker } from '@nitrots/nitro-renderer'; import { FC, useEffect, useMemo, useRef, useState } from 'react'; -import { ChatEntryType, LocalizeText } from '../../api'; +import { ChatEntryType, LocalizeText, SanitizeHtml } from '../../api'; import { Flex, NitroCardContentView, NitroCardHeaderView, NitroCardTabsItemView, NitroCardTabsView, NitroCardView, Text } from '../../common'; import { useChatHistory, useMentionActions, useMentionsSnapshot, useOnClickChat } from '../../hooks'; import { useUserDataSnapshot } from '../../hooks/session/useSessionSnapshots'; @@ -137,8 +137,8 @@ export const ChatHistoryView: FC<{}> = props => )}
- - + +
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/help/views/SelectReportedUserView.tsx b/src/components/help/views/SelectReportedUserView.tsx index 24233a7..1e40801 100644 --- a/src/components/help/views/SelectReportedUserView.tsx +++ b/src/components/help/views/SelectReportedUserView.tsx @@ -1,6 +1,6 @@ import { GetSessionDataManager, RoomObjectType } from '@nitrots/nitro-renderer'; import { FC, useMemo, useState } from 'react'; -import { ChatEntryType, IReportedUser, LocalizeText, ReportState } from '../../../api'; +import { ChatEntryType, IReportedUser, LocalizeText, ReportState, SanitizeHtml } from '../../../api'; import { AutoGrid, Button, Column, Flex, LayoutGridItem, Text } from '../../../common'; import { useChatHistory, useHelp } from '../../../hooks'; @@ -66,7 +66,7 @@ export const SelectReportedUserView: FC<{}> = props => { return ( selectUser(user.id) }> - + ); }) } diff --git a/src/components/notification-center/views/alert-layouts/NotificationDefaultAlertView.tsx b/src/components/notification-center/views/alert-layouts/NotificationDefaultAlertView.tsx index 14c289d..629ff07 100644 --- a/src/components/notification-center/views/alert-layouts/NotificationDefaultAlertView.tsx +++ b/src/components/notification-center/views/alert-layouts/NotificationDefaultAlertView.tsx @@ -1,5 +1,5 @@ import { FC, useMemo, useState } from 'react'; -import { DispatchUiEvent, LocalizeText, NotificationAlertItem, NotificationAlertType, OpenUrl, RoomWidgetUpdateChatInputContentEvent } from '../../../../api'; +import { DispatchUiEvent, LocalizeText, NotificationAlertItem, NotificationAlertType, OpenUrl, RoomWidgetUpdateChatInputContentEvent, SanitizeHtml } from '../../../../api'; import { Button, Column, Flex, LayoutNotificationAlertView, LayoutNotificationAlertViewProps } from '../../../../common'; interface NotificationDefaultAlertViewProps extends LayoutNotificationAlertViewProps @@ -97,7 +97,7 @@ export const NotificationDefaultAlertView: FC { const htmlText = message.replace(/\r\n|\r|\n/g, '
'); - return
; + return
; }) } { item.clickUrl && (item.clickUrl.length > 0) && (item.imageUrl && !imageFailed) && <>
diff --git a/src/components/notification-center/views/bubble-layouts/NotificationDefaultBubbleView.tsx b/src/components/notification-center/views/bubble-layouts/NotificationDefaultBubbleView.tsx index d011269..f7b5f43 100644 --- a/src/components/notification-center/views/bubble-layouts/NotificationDefaultBubbleView.tsx +++ b/src/components/notification-center/views/bubble-layouts/NotificationDefaultBubbleView.tsx @@ -1,5 +1,5 @@ import { FC } from 'react'; -import { NotificationBubbleItem, OpenUrl } from '../../../../api'; +import { NotificationBubbleItem, OpenUrl, SanitizeHtml } from '../../../../api'; import { Flex, LayoutNotificationBubbleView, LayoutNotificationBubbleViewProps, Text } from '../../../../common'; export interface NotificationDefaultBubbleViewProps extends LayoutNotificationBubbleViewProps @@ -19,7 +19,7 @@ export const NotificationDefaultBubbleView: FC } - + ); }; diff --git a/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetAvatarView.tsx b/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetAvatarView.tsx index d5e1c19..45d7fa6 100644 --- a/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetAvatarView.tsx +++ b/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetAvatarView.tsx @@ -1,7 +1,7 @@ import { CreateLinkEvent, FlatControllerAddedEvent, FlatControllerRemovedEvent, GetSessionDataManager, RoomControllerLevel, RoomObjectCategory, RoomObjectVariable, RoomUnitGiveHandItemComposer, SetRelationshipStatusComposer, TradingOpenComposer } from '@nitrots/nitro-renderer'; import { FC, useEffect, useMemo, useState } from 'react'; import { FaChevronLeft, FaChevronRight } from 'react-icons/fa'; -import { AvatarInfoUser, DispatchUiEvent, GetOwnRoomObject, GetUserProfile, LocalizeText, MessengerFriend, ReportType, RoomWidgetUpdateChatInputContentEvent, SendMessageComposer } from '../../../../../api'; +import { AvatarInfoUser, DispatchUiEvent, GetOwnRoomObject, GetUserProfile, LocalizeText, MessengerFriend, ReportType, RoomWidgetUpdateChatInputContentEvent, SanitizeHtml, SendMessageComposer } from '../../../../../api'; import { Flex } from '../../../../../common'; import { useFriends, useHelp, useIsUserIgnored, useMessageEvent, useRoom, useSessionInfo, useWiredTools } from '../../../../../hooks'; import { ContextMenuHeaderView } from '../../context-menu/ContextMenuHeaderView'; @@ -244,7 +244,7 @@ export const AvatarInfoWidgetAvatarView: FC = p return ( - GetUserProfile(avatarInfo.webID) } dangerouslySetInnerHTML={ { __html: `${ avatarInfo.name }` } }> + GetUserProfile(avatarInfo.webID) } dangerouslySetInnerHTML={ { __html: SanitizeHtml(`${ avatarInfo.name }`) } }> { (mode === MODE_NORMAL) && <> { canRequestFriend(avatarInfo.webID) && diff --git a/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetNameView.tsx b/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetNameView.tsx index bfa7b41..c79ca57 100644 --- a/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetNameView.tsx +++ b/src/components/room/widgets/avatar-info/menu/AvatarInfoWidgetNameView.tsx @@ -1,6 +1,6 @@ import { GetSessionDataManager } from '@nitrots/nitro-renderer'; import { FC, useMemo } from 'react'; -import { AvatarInfoName } from '../../../../../api'; +import { AvatarInfoName, SanitizeHtml } from '../../../../../api'; import { ContextMenuView } from '../../context-menu/ContextMenuView'; interface AvatarInfoWidgetNameViewProps @@ -24,7 +24,7 @@ export const AvatarInfoWidgetNameView: FC = props return ( -
+
); }; diff --git a/src/components/room/widgets/chat/ChatWidgetWindowView.tsx b/src/components/room/widgets/chat/ChatWidgetWindowView.tsx index 7734bd8..c756ccc 100644 --- a/src/components/room/widgets/chat/ChatWidgetWindowView.tsx +++ b/src/components/room/widgets/chat/ChatWidgetWindowView.tsx @@ -1,6 +1,6 @@ import { GetSessionDataManager, RoomObjectType } from '@nitrots/nitro-renderer'; import { FC, UIEvent, useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { ChatEntryType, LocalizeText } from '../../../../api'; +import { ChatEntryType, LocalizeText, SanitizeHtml } from '../../../../api'; import { DraggableWindowPosition, NitroCardContentView, NitroCardHeaderView, NitroCardView } from '../../../../common'; import { useChatHistory, useChatWindow, useOnClickChat } from '../../../../hooks'; import { useRoom } from '../../../../hooks/rooms'; @@ -133,18 +133,18 @@ export const ChatWidgetWindowView: FC<{}> = () => { hideBalloons && !hideAvatars &&
} { hideBalloons && (
- + { !chat.showTranslation && - } + } { chat.showTranslation &&
original: - +
translate: - +
}
@@ -161,18 +161,18 @@ export const ChatWidgetWindowView: FC<{}> = () => ) }
- + { !chat.showTranslation && - } + } { chat.showTranslation &&
original: - +
translate: - +
}
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); };