From 301294ecf4a395fafdb60d5b729b9252699e9e45 Mon Sep 17 00:00:00 2001 From: simoleo89 <11816867+simoleo89@users.noreply.github.com> Date: Wed, 17 Jun 2026 19:00:42 +0200 Subject: [PATCH] fix(security): sanitize user-controlled HTML in chat & username sinks Several dangerouslySetInnerHTML sinks rendered user-controlled strings (chat messages, usernames, chat history) without sanitisation, relying implicitly on upstream formatting or server-side charset limits. Route them all through the existing SanitizeHtml (DOMPurify) helper so the security guarantee is local to each render site. Sinks fixed: ChatWidgetWindowView (name/message/original/translated), ChatHistoryView (name/message), AvatarInfoWidgetNameView + AvatarInfoWidgetAvatarView (username), SelectReportedUserView (username). Add regression suites: SanitizeHtml.test.ts (XSS neutralised, chat markup preserved) and RoomChatFormatter.test.ts (pins the existing encodeHTML defence). No behaviour change: SanitizeHtml's allow-list keeps the b/i/u/span/strong/em/br markup the chat/profile UI relies on. --- src/api/utils/RoomChatFormatter.test.ts | 112 ++++++++++++++++++ src/api/utils/SanitizeHtml.test.ts | 88 ++++++++++++++ .../chat-history/ChatHistoryView.tsx | 6 +- .../help/views/SelectReportedUserView.tsx | 4 +- .../menu/AvatarInfoWidgetAvatarView.tsx | 4 +- .../menu/AvatarInfoWidgetNameView.tsx | 4 +- .../widgets/chat/ChatWidgetWindowView.tsx | 18 +-- 7 files changed, 218 insertions(+), 18 deletions(-) create mode 100644 src/api/utils/RoomChatFormatter.test.ts create mode 100644 src/api/utils/SanitizeHtml.test.ts 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..656d00b --- /dev/null +++ b/src/api/utils/SanitizeHtml.test.ts @@ -0,0 +1,88 @@ +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); + }); +}); 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/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/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: - +
}