mirror of
https://github.com/duckietm/Nitro-V3.git
synced 2026-06-19 15:06:20 +00:00
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.
This commit is contained in:
simoleo89
@@ -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 <script>/<img onerror>/<svg onload>/javascript: URL,
|
||||
* no event-handler attributes). We use jsdom's real HTML parser as the oracle
|
||||
* rather than guessing how entities decode.
|
||||
*/
|
||||
const parse = (input: string): HTMLDivElement =>
|
||||
{
|
||||
const div = document.createElement('div');
|
||||
div.innerHTML = RoomChatFormatter(input);
|
||||
return div;
|
||||
};
|
||||
|
||||
describe('RoomChatFormatter — XSS neutralisation', () =>
|
||||
{
|
||||
it('does not produce a <script> element from a raw script tag', () =>
|
||||
{
|
||||
const div = parse('<script>alert(1)</script>');
|
||||
expect(div.querySelector('script')).toBeNull();
|
||||
});
|
||||
|
||||
it('does not produce an <img> element with an onerror handler', () =>
|
||||
{
|
||||
const div = parse('<img src=x onerror=alert(1)>');
|
||||
const img = div.querySelector('img');
|
||||
expect(img).toBeNull();
|
||||
});
|
||||
|
||||
it('does not produce an <svg> element with an onload handler', () =>
|
||||
{
|
||||
const div = parse('<svg onload=alert(1)></svg>');
|
||||
expect(div.querySelector('svg')).toBeNull();
|
||||
});
|
||||
|
||||
it('does not keep a javascript: href on an anchor', () =>
|
||||
{
|
||||
const div = parse('<a href="javascript:alert(1)">x</a>');
|
||||
expect(div.querySelector('a[href^="javascript:"]')).toBeNull();
|
||||
});
|
||||
|
||||
it('strips event-handler attributes injected via a font tag', () =>
|
||||
{
|
||||
const div = parse('<font color="red" onload="alert(1)">hi</font>');
|
||||
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('<font color="red;background:url(javascript:alert(1))">hi</font>');
|
||||
expect(div.innerHTML.toLowerCase()).not.toContain('javascript:');
|
||||
});
|
||||
});
|
||||
|
||||
describe('RoomChatFormatter — legitimate markup is preserved', () =>
|
||||
{
|
||||
it('renders [b]…[/b] as <strong>', () =>
|
||||
{
|
||||
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('<font color="red">hi</font>');
|
||||
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('<font color="notacolour">hi</font>');
|
||||
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 <br />', () =>
|
||||
{
|
||||
const div = parse('line1\nline2');
|
||||
expect(div.querySelectorAll('br').length).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -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 <script> elements', () =>
|
||||
{
|
||||
expect(parse('<script>alert(1)</script>').querySelector('script')).toBeNull();
|
||||
});
|
||||
|
||||
it('strips inline event handlers (onerror) from allowed tags', () =>
|
||||
{
|
||||
const div = parse('<img src=x onerror=alert(1)>');
|
||||
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('<a href="javascript:alert(1)">x</a>');
|
||||
expect(div.querySelector('a[href^="javascript:"]')).toBeNull();
|
||||
});
|
||||
|
||||
it('removes <svg> and its onload handler', () =>
|
||||
{
|
||||
const div = parse('<svg onload=alert(1)></svg>');
|
||||
expect(div.innerHTML.toLowerCase()).not.toContain('onload');
|
||||
});
|
||||
|
||||
it('removes <iframe> elements', () =>
|
||||
{
|
||||
expect(parse('<iframe src="https://evil.example"></iframe>').querySelector('iframe')).toBeNull();
|
||||
});
|
||||
|
||||
it('leaves a plain username untouched', () =>
|
||||
{
|
||||
expect(SanitizeHtml('CoolUser_123')).toBe('CoolUser_123');
|
||||
});
|
||||
});
|
||||
|
||||
describe('SanitizeHtml — preserves the markup the chat/profile UI relies on', () =>
|
||||
{
|
||||
it('keeps <b>/<i>/<u>', () =>
|
||||
{
|
||||
const div = parse('<b>a</b><i>b</i><u>c</u>');
|
||||
expect(div.querySelector('b')).not.toBeNull();
|
||||
expect(div.querySelector('i')).not.toBeNull();
|
||||
expect(div.querySelector('u')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('keeps <strong>/<em> (RoomChatFormatter output)', () =>
|
||||
{
|
||||
const div = parse('<strong>x</strong><em>y</em>');
|
||||
expect(div.querySelector('strong')).not.toBeNull();
|
||||
expect(div.querySelector('em')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('keeps a coloured span (RoomChatFormatter output)', () =>
|
||||
{
|
||||
const div = parse('<span style="color:red">hi</span>');
|
||||
const span = div.querySelector('span');
|
||||
expect(span).not.toBeNull();
|
||||
expect((span as HTMLElement).style.color).toBe('red');
|
||||
});
|
||||
|
||||
it('keeps <br>', () =>
|
||||
{
|
||||
expect(parse('a<br />b').querySelectorAll('br').length).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -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 =>
|
||||
)}
|
||||
</div>
|
||||
<div className="chat-content">
|
||||
<b className="mr-1 username" dangerouslySetInnerHTML={{ __html: `${row.name}: ` }} />
|
||||
<span className="message" dangerouslySetInnerHTML={{ __html: `${row.message}` }} onClick={ onClickChat } />
|
||||
<b className="mr-1 username" dangerouslySetInnerHTML={{ __html: SanitizeHtml(`${row.name}: `) }} />
|
||||
<span className="message" dangerouslySetInnerHTML={{ __html: SanitizeHtml(`${row.message}`) }} onClick={ onClickChat } />
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -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 (
|
||||
<LayoutGridItem key={ user.id } itemActive={ (selectedUserId === user.id) } onClick={ event => selectUser(user.id) }>
|
||||
<span dangerouslySetInnerHTML={ { __html: (user.username) } } />
|
||||
<span dangerouslySetInnerHTML={ { __html: SanitizeHtml(user.username) } } />
|
||||
</LayoutGridItem>
|
||||
);
|
||||
}) }
|
||||
|
||||
@@ -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<AvatarInfoWidgetAvatarViewProps> = p
|
||||
|
||||
return (
|
||||
<ContextMenuView category={ RoomObjectCategory.UNIT } classNames={ [ 'nitro-avatar-action-menu' ] } collapsable={ true } objectId={ avatarInfo.roomIndex } userType={ avatarInfo.userType } onClose={ onClose }>
|
||||
<ContextMenuHeaderView className="cursor-pointer" onClick={ event => GetUserProfile(avatarInfo.webID) } dangerouslySetInnerHTML={ { __html: `${ avatarInfo.name }` } }></ContextMenuHeaderView>
|
||||
<ContextMenuHeaderView className="cursor-pointer" onClick={ event => GetUserProfile(avatarInfo.webID) } dangerouslySetInnerHTML={ { __html: SanitizeHtml(`${ avatarInfo.name }`) } }></ContextMenuHeaderView>
|
||||
{ (mode === MODE_NORMAL) &&
|
||||
<>
|
||||
{ canRequestFriend(avatarInfo.webID) &&
|
||||
|
||||
@@ -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<AvatarInfoWidgetNameViewProps> = props
|
||||
|
||||
return (
|
||||
<ContextMenuView category={ nameInfo.category } classNames={ getClassNames } fades={ (nameInfo.id !== GetSessionDataManager().userId) } objectId={ nameInfo.roomIndex } userType={ nameInfo.userType } onClose={ onClose }>
|
||||
<div className="text-shadow" dangerouslySetInnerHTML={ { __html: `${ nameInfo.name }` } }></div>
|
||||
<div className="text-shadow" dangerouslySetInnerHTML={ { __html: SanitizeHtml(`${ nameInfo.name }`) } }></div>
|
||||
</ContextMenuView>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -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 && <div className={ `w-[65px] h-[55px] shrink-0 mt-[-18px] rounded-sm bg-no-repeat bg-center scale-70 ${ isOwnMessage ? 'order-2' : '' }` } style={ chat.imageUrl ? { backgroundImage: `url(${ chat.imageUrl })` } : undefined } /> }
|
||||
{ hideBalloons && (
|
||||
<div onClick={ onClickChat }>
|
||||
<b dangerouslySetInnerHTML={ { __html: `${ chat.name }: ` } } />
|
||||
<b dangerouslySetInnerHTML={ { __html: SanitizeHtml(`${ chat.name }: `) } } />
|
||||
{ !chat.showTranslation &&
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: chat.message } } /> }
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: SanitizeHtml(chat.message) } } /> }
|
||||
{ chat.showTranslation &&
|
||||
<div className="mt-[2px] flex flex-col gap-[2px]">
|
||||
<div className="flex items-start gap-1 leading-[1.15]">
|
||||
<span className="inline-block min-w-[52px] font-bold opacity-75">original:</span>
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: chat.originalMessage || chat.message || '' } } />
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: SanitizeHtml(chat.originalMessage || chat.message || '') } } />
|
||||
</div>
|
||||
<div className="flex items-start gap-1 leading-[1.15]">
|
||||
<span className="inline-block min-w-[52px] font-bold opacity-75">translate:</span>
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: chat.translatedMessage || chat.message || '' } } />
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: SanitizeHtml(chat.translatedMessage || chat.message || '') } } />
|
||||
</div>
|
||||
</div> }
|
||||
</div>
|
||||
@@ -161,18 +161,18 @@ export const ChatWidgetWindowView: FC<{}> = () =>
|
||||
) }
|
||||
</div>
|
||||
<div className={ `chat-content py-[5px] px-[6px] leading-none min-h-[25px] ${ !hideAvatars ? (isOwnMessage ? 'mr-[27px]' : 'ml-[27px]') : '' }` }>
|
||||
<b className="username" dangerouslySetInnerHTML={ { __html: `${ chat.name }: ` } } />
|
||||
<b className="username" dangerouslySetInnerHTML={ { __html: SanitizeHtml(`${ chat.name }: `) } } />
|
||||
{ !chat.showTranslation &&
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: `${ chat.message }` } } onClick={ onClickChat } /> }
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: SanitizeHtml(`${ chat.message }`) } } onClick={ onClickChat } /> }
|
||||
{ chat.showTranslation &&
|
||||
<div className="mt-[2px] flex flex-col gap-[2px]" onClick={ onClickChat }>
|
||||
<div className="flex items-start gap-1 leading-[1.1]">
|
||||
<span className="inline-block min-w-[52px] font-bold" style={ { opacity: 0.75 } }>original:</span>
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: `${ chat.originalMessage || chat.message || '' }` } } />
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: SanitizeHtml(`${ chat.originalMessage || chat.message || '' }`) } } />
|
||||
</div>
|
||||
<div className="flex items-start gap-1 leading-[1.1]">
|
||||
<span className="inline-block min-w-[52px] font-bold" style={ { opacity: 0.75 } }>translate:</span>
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: `${ chat.translatedMessage || chat.message || '' }` } } />
|
||||
<span className={ messageClassName } dangerouslySetInnerHTML={ { __html: SanitizeHtml(`${ chat.translatedMessage || chat.message || '' }`) } } />
|
||||
</div>
|
||||
</div> }
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user