From b500afc8a98c451719338454296ffba69b5cac3b Mon Sep 17 00:00:00 2001 From: heaven Date: Fri, 12 Jun 2026 13:04:19 +0300 Subject: [PATCH] fix: harden client against crafted-event crashes and audit-found hardening gaps --- .github/workflows/deploy-pull-request.yml | 21 ++++++++-- electron-builder.json | 1 + electron/afterPack.cjs | 42 +++++++++++++++++++ electron/main.ts | 42 ++++++++++++++----- package-lock.json | 15 +++---- package.json | 4 ++ public/locales/en.json | 1 + public/locales/ru.json | 1 + src/app/components/editor/input.ts | 14 ++++--- src/app/components/message/Reply.tsx | 6 ++- .../call-status/IncomingCallStrip.tsx | 11 ++++- src/app/features/room-nav/DmStreamRow.tsx | 28 ++++++++++--- src/app/features/room-nav/RoomNavItem.tsx | 24 ++++++++--- .../features/room/MessageErrorBoundary.tsx | 38 +++++++++++++++++ src/app/features/room/RoomTimeline.tsx | 11 ++++- src/app/features/room/ThreadDrawer.tsx | 11 ++++- src/app/hooks/useIncomingRtcNotifications.ts | 6 +-- src/app/hooks/usePowerLevels.ts | 2 +- src/app/pages/Router.tsx | 6 ++- src/app/plugins/call/CallWidgetDriver.ts | 19 ++++++++- src/app/utils/audioWaveform.ts | 17 ++++++-- src/app/utils/room.ts | 1 + src/app/utils/rtcNotification.ts | 22 +++++++++- src/client/initMatrix.ts | 19 ++++++++- 24 files changed, 303 insertions(+), 59 deletions(-) create mode 100644 electron/afterPack.cjs create mode 100644 src/app/features/room/MessageErrorBoundary.tsx diff --git a/.github/workflows/deploy-pull-request.yml b/.github/workflows/deploy-pull-request.yml index a4fc308a..c21e807a 100644 --- a/.github/workflows/deploy-pull-request.yml +++ b/.github/workflows/deploy-pull-request.yml @@ -21,9 +21,19 @@ jobs: workflow: ${{ github.event.workflow.id }} run_id: ${{ github.event.workflow_run.id }} name: pr - - name: Output pr number + - name: Validate and output pr number id: pr - run: echo "id=$(> $GITHUB_OUTPUT + # pr.txt comes from an untrusted fork-PR build artifact. Validate it is + # purely numeric before it reaches $GITHUB_OUTPUT, otherwise embedded + # newlines could inject arbitrary step outputs (pwn-request). Mirrors + # upstream security fix 64468dfb. + run: | + PR_ID=$(> "${GITHUB_OUTPUT}" - name: Download artifact uses: dawidd6/action-download-artifact@2536c51d3d126276eb39f74d6bc9c72ac6ef30d3 # v16 with: @@ -42,7 +52,12 @@ jobs: enable-pull-request-comment: false enable-commit-comment: false env: - NETLIFY_AUTH_TOKEN: ${{ secrets.NETLIFY_AUTH_TOKEN }} + # This deploys an artifact built from an untrusted fork PR. Prefer a + # least-privilege token scoped only to the PR-preview site over the + # production NETLIFY_AUTH_TOKEN — provision NETLIFY_AUTH_TOKEN_PR and + # switch to it (upstream 64468dfb). Left on the shared token until that + # secret exists so previews keep working; see audit MEDIUM finding. + NETLIFY_AUTH_TOKEN: ${{ secrets.NETLIFY_AUTH_TOKEN_PR || secrets.NETLIFY_AUTH_TOKEN }} NETLIFY_SITE_ID: ${{ secrets.NETLIFY_SITE_ID_PR_VOJO }} timeout-minutes: 1 - name: Comment preview on PR diff --git a/electron-builder.json b/electron-builder.json index e0f41849..25d65edc 100644 --- a/electron-builder.json +++ b/electron-builder.json @@ -2,6 +2,7 @@ "appId": "chat.vojo.desktop", "productName": "Vojo", "asar": true, + "afterPack": "electron/afterPack.cjs", "directories": { "output": "release" }, diff --git a/electron/afterPack.cjs b/electron/afterPack.cjs new file mode 100644 index 00000000..ce4c9a89 --- /dev/null +++ b/electron/afterPack.cjs @@ -0,0 +1,42 @@ +/* eslint-disable @typescript-eslint/no-var-requires -- electron-builder loads this hook as a CommonJS module; require is required. */ +// electron-builder afterPack hook: harden Electron fuses on the packaged binary. +// +// By default Electron ships with several fuses ON that let a local process turn +// the signed app into a generic Node interpreter or attach a debugger: +// - RunAsNode (ELECTRON_RUN_AS_NODE) +// - EnableNodeCliInspectArguments (--inspect) +// - EnableNodeOptionsEnvironmentVariable (NODE_OPTIONS injection) +// We flip them OFF. OnlyLoadAppFromAsar + EnableEmbeddedAsarIntegrityValidation +// are intentionally left for later: integrity validation is only meaningful once +// the binary is code-signed (no signing in the current build chain). +const path = require('node:path'); +const { existsSync } = require('node:fs'); +const { flipFuses, FuseVersion, FuseV1Options } = require('@electron/fuses'); + +exports.default = async function afterPack(context) { + const { appOutDir, packager, electronPlatformName } = context; + const productName = packager.appInfo.productFilename; + const isMac = electronPlatformName === 'darwin' || electronPlatformName === 'mas'; + + let electronBinary; + if (isMac) { + electronBinary = path.join(appOutDir, `${productName}.app`, 'Contents', 'MacOS', productName); + } else if (electronPlatformName === 'win32') { + electronBinary = path.join(appOutDir, `${productName}.exe`); + } else { + electronBinary = path.join(appOutDir, packager.executableName || productName); + } + + if (!existsSync(electronBinary)) { + throw new Error(`afterPack: could not find Electron binary to harden at ${electronBinary}`); + } + + await flipFuses(electronBinary, { + version: FuseVersion.V1, + // Re-applies the ad-hoc signature macOS needs after the binary is mutated. + resetAdHocDarwinSignature: isMac, + [FuseV1Options.RunAsNode]: false, + [FuseV1Options.EnableNodeCliInspectArguments]: false, + [FuseV1Options.EnableNodeOptionsEnvironmentVariable]: false, + }); +}; diff --git a/electron/main.ts b/electron/main.ts index 1f8e0e43..4947c07a 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -70,6 +70,21 @@ const isSafeExternal = (raw: unknown): raw is string => { } }; +// The canonical in-app origin (prod custom scheme, or the dev server in dev). +// Used by both the navigation guard and the permission handlers. +const isInternalUrl = (raw: unknown): raw is string => { + if (typeof raw !== 'string') return false; + try { + const target = new URL(raw); + return ( + (target.protocol === `${APP_SCHEME}:` && target.hostname === APP_HOST) || + (isDev && target.origin === DEV_URL) + ); + } catch { + return false; + } +}; + const distDir = path.resolve(__dirname, '..', '..', 'dist'); // React Router defaults to BrowserRouter against `window.location.pathname`, @@ -218,25 +233,30 @@ const createWindow = async () => { }); win.webContents.on('will-navigate', (event, url) => { - let target: URL; - try { - target = new URL(url); - } catch { - event.preventDefault(); - return; - } // Strict match for the in-app origin. `target.protocol === 'vojo:'` alone // would treat `vojo://evil/...` as internal even though the protocol // handler now rejects it; preventing navigation upstream avoids the // round-trip and keeps the renderer pinned to the canonical origin. - const isInternal = - (target.protocol === `${APP_SCHEME}:` && target.hostname === APP_HOST) || - (isDev && target.origin === DEV_URL); - if (isInternal) return; + if (isInternalUrl(url)) return; event.preventDefault(); if (isSafeExternal(url)) shell.openExternal(url); }); + // Electron auto-approves EVERY permission request unless a handler is set. + // Deny by default and allow only what the app legitimately needs (media for + // calls, notifications) and only from the in-app origin — otherwise a DOM XSS + // in the renderer could silently call getUserMedia/geolocation with no consent + // prompt (unlike the browser build). + const ALLOWED_PERMISSIONS = new Set(['media', 'notifications']); + const { session } = win.webContents; + session.setPermissionRequestHandler((_wc, permission, callback, details) => { + callback(isInternalUrl(details.requestingUrl) && ALLOWED_PERMISSIONS.has(permission)); + }); + session.setPermissionCheckHandler( + (_wc, permission, requestingOrigin) => + isInternalUrl(requestingOrigin) && ALLOWED_PERMISSIONS.has(permission) + ); + if (isDev) { await win.loadURL(DEV_URL); win.webContents.openDevTools({ mode: 'detach' }); diff --git a/package-lock.json b/package-lock.json index 63c613ba..59a4df47 100644 --- a/package-lock.json +++ b/package-lock.json @@ -75,6 +75,7 @@ "ua-parser-js": "1.0.35" }, "devDependencies": { + "@electron/fuses": "1.8.0", "@element-hq/element-call-embedded": "0.16.3", "@esbuild-plugins/node-globals-polyfill": "0.2.3", "@rollup/plugin-inject": "5.0.3", @@ -12038,9 +12039,10 @@ } }, "node_modules/lodash": { - "version": "4.17.21", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", - "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" + "version": "4.18.1", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.18.1.tgz", + "integrity": "sha512-dMInicTPVE8d1e5otfwmmjlxkZoUpiVLwyeTdUsi/Caj/gfzzblBcCE5sRHV/AsjuCmxWrte2TNGSYuCeCq+0Q==", + "license": "MIT" }, "node_modules/lodash.debounce": { "version": "4.0.8", @@ -16526,13 +16528,6 @@ "node": ">=20.0.0" } }, - "node_modules/wait-on/node_modules/lodash": { - "version": "4.18.1", - "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.18.1.tgz", - "integrity": "sha512-dMInicTPVE8d1e5otfwmmjlxkZoUpiVLwyeTdUsi/Caj/gfzzblBcCE5sRHV/AsjuCmxWrte2TNGSYuCeCq+0Q==", - "dev": true, - "license": "MIT" - }, "node_modules/wcwidth": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/wcwidth/-/wcwidth-1.0.1.tgz", diff --git a/package.json b/package.json index 77b0a441..2e80b0f8 100644 --- a/package.json +++ b/package.json @@ -118,6 +118,7 @@ "ua-parser-js": "1.0.35" }, "devDependencies": { + "@electron/fuses": "1.8.0", "@element-hq/element-call-embedded": "0.16.3", "@esbuild-plugins/node-globals-polyfill": "0.2.3", "@rollup/plugin-inject": "5.0.3", @@ -157,5 +158,8 @@ "vite-plugin-static-copy": "1.0.4", "vite-plugin-top-level-await": "1.4.4", "wait-on": "9.0.10" + }, + "overrides": { + "lodash": "4.18.1" } } diff --git a/public/locales/en.json b/public/locales/en.json index 92de94d3..74c4dbfc 100644 --- a/public/locales/en.json +++ b/public/locales/en.json @@ -537,6 +537,7 @@ "jump_to_unread": "Jump to Unread", "mark_as_read": "Mark as Read", "jump_to_latest": "Jump to Latest", + "message_render_error": "This message could not be displayed.", "today": "Today", "yesterday": "Yesterday", "view_reactions": "View Reactions", diff --git a/public/locales/ru.json b/public/locales/ru.json index c287e293..e29567c1 100644 --- a/public/locales/ru.json +++ b/public/locales/ru.json @@ -543,6 +543,7 @@ "jump_to_unread": "К непрочитанным", "mark_as_read": "Отметить прочитанным", "jump_to_latest": "К последним", + "message_render_error": "Не удалось показать это сообщение.", "today": "Сегодня", "yesterday": "Вчера", "view_reactions": "Реакции", diff --git a/src/app/components/editor/input.ts b/src/app/components/editor/input.ts index 4b6df40f..1af30a7f 100644 --- a/src/app/components/editor/input.ts +++ b/src/app/components/editor/input.ts @@ -157,10 +157,12 @@ const getInlineElement = (node: ChildNode, processText: ProcessTextCallback): In return children; } - return node.childNodes.flatMap((child) => getInlineElement(child, processText)); + const children = node.childNodes.flatMap((child) => getInlineElement(child, processText)); + if (children.length === 0) return [{ text: '' }]; + return children; } - return []; + return [{ text: '' }]; }; const parseBlockquoteNode = ( @@ -191,7 +193,7 @@ const parseBlockquoteNode = ( if (child.name === 'p') { appendLine(); - quoteLines.push(child.children.flatMap((c) => getInlineElement(c, processText))); + quoteLines.push(getInlineElement(child, processText)); return; } @@ -281,7 +283,7 @@ const parseListNode = ( if (child.name === 'li') { appendLine(); - listLines.push(child.children.flatMap((c) => getInlineElement(c, processText))); + listLines.push(getInlineElement(child, processText)); return; } @@ -329,7 +331,7 @@ const parseHeadingNode = ( node: Element, processText: ProcessTextCallback ): HeadingElement | ParagraphElement => { - const children = node.children.flatMap((child) => getInlineElement(child, processText)); + const children = getInlineElement(node, processText); const headingMatch = node.name.match(/^h([123456])$/); const [, g1AsLevel] = headingMatch ?? ['h3', '3']; @@ -392,7 +394,7 @@ export const domToEditorInput = ( appendLine(); children.push({ type: BlockType.Paragraph, - children: node.children.flatMap((child) => getInlineElement(child, processText)), + children: getInlineElement(node, processText), }); return; } diff --git a/src/app/components/message/Reply.tsx b/src/app/components/message/Reply.tsx index 536a0698..c2eef262 100644 --- a/src/app/components/message/Reply.tsx +++ b/src/app/components/message/Reply.tsx @@ -106,7 +106,11 @@ export const Reply = as<'div', ReplyProps>( ); const badEncryption = replyEvent?.getContent().msgtype === 'm.bad.encrypted'; - const bodyJSX = body ? scaleSystemEmoji(trimReplyFromBody(body)) : fallbackBody; + // `body` is attacker-controlled event content and may be any JSON type, not + // the string the type claims. Guard before string parsing — a non-string + // body would make trimReplyFromBody (`body.match`) throw during render. + const bodyJSX = + typeof body === 'string' && body ? scaleSystemEmoji(trimReplyFromBody(body)) : fallbackBody; return ( diff --git a/src/app/features/call-status/IncomingCallStrip.tsx b/src/app/features/call-status/IncomingCallStrip.tsx index 0076f494..d2d2c937 100644 --- a/src/app/features/call-status/IncomingCallStrip.tsx +++ b/src/app/features/call-status/IncomingCallStrip.tsx @@ -38,6 +38,10 @@ export function IncomingCallStrip({ call, room }: IncomingCallStripProps) { (senderId && getMxIdLocalPart(senderId)) || senderId || t('Call.unknown_caller'); + // Display names are peer-controlled and spoofable ("Mom", "IT Support"). On a + // high-trust ring surface, also surface the unspoofable mxid whenever it isn't + // already what we're showing, so the user can verify who is really calling. + const senderMxidLabel = senderId && displayName !== senderId ? senderId : undefined; const avatarMxc = senderId ? getMemberAvatarMxc(room, senderId) : undefined; const avatarUrl = avatarMxc ? mxcUrlToHttp(mx, avatarMxc, useAuthentication, 96, 96, 'crop') ?? undefined @@ -143,6 +147,11 @@ export function IncomingCallStrip({ call, room }: IncomingCallStripProps) { {displayName} + {senderMxidLabel && ( + + {senderMxidLabel} + + )} {t('Call.incoming_label')} @@ -166,7 +175,7 @@ export function IncomingCallStrip({ call, room }: IncomingCallStripProps) { {displayName} - {t('Call.incoming')} + {senderMxidLabel ?? t('Call.incoming')} diff --git a/src/app/features/room-nav/DmStreamRow.tsx b/src/app/features/room-nav/DmStreamRow.tsx index da0b1208..918c6ed8 100644 --- a/src/app/features/room-nav/DmStreamRow.tsx +++ b/src/app/features/room-nav/DmStreamRow.tsx @@ -24,12 +24,17 @@ import { useAtom, useAtomValue } from 'jotai'; import { NavItem, NavItemContent, NavItemOptions, NavLink } from '../../components/nav'; import { UnreadBadge, UnreadBadgeCenter } from '../../components/unread-badge'; import { RoomAvatar } from '../../components/room-avatar'; -import { getDirectRoomAvatarUrl, getRoomAvatarUrl, isOneOnOneRoom } from '../../utils/room'; +import { + getDirectRoomAvatarUrl, + getRoomAvatarUrl, + getStateEvent, + isOneOnOneRoom, +} from '../../utils/room'; import { nameInitials } from '../../utils/common'; import { useMatrixClient } from '../../hooks/useMatrixClient'; import { useRoomUnread } from '../../state/hooks/unread'; import { roomToUnreadAtom } from '../../state/room/roomToUnread'; -import { usePowerLevels } from '../../hooks/usePowerLevels'; +import { getPowersLevelFromMatrixEvent, usePowerLevels } from '../../hooks/usePowerLevels'; import { copyToClipboard } from '../../utils/dom'; import { markAsRead } from '../../utils/notifications'; import { UseStateProvider } from '../../components/UseStateProvider'; @@ -50,8 +55,8 @@ import { getRoomNotificationModeIcon, } from '../../hooks/useRoomsNotificationPreferences'; import { RoomNotificationModeSwitcher } from '../../components/RoomNotificationSwitcher'; -import { useRoomCreators } from '../../hooks/useRoomCreators'; -import { useRoomPermissions } from '../../hooks/useRoomPermissions'; +import { getRoomCreatorsForRoomId, useRoomCreators } from '../../hooks/useRoomCreators'; +import { getRoomPermissionsAPI, useRoomPermissions } from '../../hooks/useRoomPermissions'; import { InviteUserPrompt } from '../../components/invite-user-prompt'; import { useRoomName } from '../../hooks/useRoomMeta'; import { useCallMembers, useCallSession } from '../../hooks/useCall'; @@ -60,6 +65,7 @@ import { callChatAtom } from '../../state/callEmbed'; import { useCallPreferencesAtom } from '../../state/hooks/callPreferences'; import { useAutoDiscoveryInfo } from '../../hooks/useAutoDiscoveryInfo'; import { livekitSupport } from '../../hooks/useLivekitSupport'; +import { StateEvent } from '../../../types/matrix/room'; import { Presence, useUserPresence } from '../../hooks/useUserPresence'; import { timeHourMinute } from '../../utils/time'; import * as css from './styles.css'; @@ -336,7 +342,19 @@ export function DmStreamRow({ room, selected, notificationMode, linkPath }: DmSt const autoDiscoveryInfo = useAutoDiscoveryInfo(); const handleStartCall: MouseEventHandler = (evt) => { - if (!livekitSupport(autoDiscoveryInfo) && callMembers.length === 0) { + // Read call permission only when actually starting a call (not reactively). + // Mirrors RoomNavItem.handleStartCall — both are call-start sinks and + // useCallStart/createCallEmbed enforce no permission of their own. + const powerLevelsEvent = getStateEvent(room, StateEvent.RoomPowerLevels); + const powerLevels = getPowersLevelFromMatrixEvent(powerLevelsEvent); + const creators = getRoomCreatorsForRoomId(mx, room.roomId); + const permissions = getRoomPermissionsAPI(creators, powerLevels); + const hasCallPermission = permissions.event( + StateEvent.GroupCallMemberPrefix, + mx.getSafeUserId() + ); + + if (!hasCallPermission || (!livekitSupport(autoDiscoveryInfo) && callMembers.length === 0)) { return; } if (callEmbed) { diff --git a/src/app/features/room-nav/RoomNavItem.tsx b/src/app/features/room-nav/RoomNavItem.tsx index 5b1a33b4..4728901e 100644 --- a/src/app/features/room-nav/RoomNavItem.tsx +++ b/src/app/features/room-nav/RoomNavItem.tsx @@ -24,12 +24,12 @@ import { useAtom, useAtomValue } from 'jotai'; import { NavItem, NavItemContent, NavItemOptions, NavLink } from '../../components/nav'; import { UnreadBadge, UnreadBadgeCenter } from '../../components/unread-badge'; import { RoomAvatar, RoomIcon } from '../../components/room-avatar'; -import { getDirectRoomAvatarUrl, getRoomAvatarUrl } from '../../utils/room'; +import { getDirectRoomAvatarUrl, getRoomAvatarUrl, getStateEvent } from '../../utils/room'; import { nameInitials } from '../../utils/common'; import { useMatrixClient } from '../../hooks/useMatrixClient'; import { useRoomUnread } from '../../state/hooks/unread'; import { roomToUnreadAtom } from '../../state/room/roomToUnread'; -import { usePowerLevels } from '../../hooks/usePowerLevels'; +import { getPowersLevelFromMatrixEvent, usePowerLevels } from '../../hooks/usePowerLevels'; import { copyToClipboard } from '../../utils/dom'; import { markAsRead } from '../../utils/notifications'; import { UseStateProvider } from '../../components/UseStateProvider'; @@ -50,8 +50,8 @@ import { RoomNotificationMode, } from '../../hooks/useRoomsNotificationPreferences'; import { RoomNotificationModeSwitcher } from '../../components/RoomNotificationSwitcher'; -import { useRoomCreators } from '../../hooks/useRoomCreators'; -import { useRoomPermissions } from '../../hooks/useRoomPermissions'; +import { getRoomCreatorsForRoomId, useRoomCreators } from '../../hooks/useRoomCreators'; +import { getRoomPermissionsAPI, useRoomPermissions } from '../../hooks/useRoomPermissions'; import { InviteUserPrompt } from '../../components/invite-user-prompt'; import { useRoomName } from '../../hooks/useRoomMeta'; import { useCallMembers, useCallSession } from '../../hooks/useCall'; @@ -60,6 +60,7 @@ import { callChatAtom } from '../../state/callEmbed'; import { useCallPreferencesAtom } from '../../state/hooks/callPreferences'; import { useAutoDiscoveryInfo } from '../../hooks/useAutoDiscoveryInfo'; import { livekitSupport } from '../../hooks/useLivekitSupport'; +import { StateEvent } from '../../../types/matrix/room'; type RoomNavItemMenuProps = { room: Room; @@ -291,8 +292,19 @@ export function RoomNavItem({ const autoDiscoveryInfo = useAutoDiscoveryInfo(); const handleStartCall: MouseEventHandler = (evt) => { - // Do not join if no livekit support or call is not started by others - if (!livekitSupport(autoDiscoveryInfo) && callMembers.length === 0) { + // Read call permission only when actually starting a call (not reactively). + const powerLevelsEvent = getStateEvent(room, StateEvent.RoomPowerLevels); + const powerLevels = getPowersLevelFromMatrixEvent(powerLevelsEvent); + const creators = getRoomCreatorsForRoomId(mx, room.roomId); + const permissions = getRoomPermissionsAPI(creators, powerLevels); + const hasCallPermission = permissions.event( + StateEvent.GroupCallMemberPrefix, + mx.getSafeUserId() + ); + + // Do not join if missing permission, or no livekit support and call is not + // started by others. + if (!hasCallPermission || (!livekitSupport(autoDiscoveryInfo) && callMembers.length === 0)) { return; } diff --git a/src/app/features/room/MessageErrorBoundary.tsx b/src/app/features/room/MessageErrorBoundary.tsx new file mode 100644 index 00000000..88e55658 --- /dev/null +++ b/src/app/features/room/MessageErrorBoundary.tsx @@ -0,0 +1,38 @@ +import React, { ReactNode } from 'react'; +import { ErrorBoundary } from 'react-error-boundary'; +import { Box, Text, color, config } from 'folds'; +import { useTranslation } from 'react-i18next'; + +function MessageRenderFallback() { + const { t } = useTranslation('Room'); + return ( + + + {t('message_render_error')} + + + ); +} + +/** + * Isolates a single timeline row's render. A hostile event (e.g. a crafted + * voice waveform or a non-string reply body) that throws during render is + * contained to one "could not display message" row instead of unmounting the + * whole React root (white-screen / persistent DoS). `resetKey` is the event id: + * it stays constant across edits/redactions of the SAME event (so a row that + * deterministically throws stays on the fallback rather than thrashing), and + * changes only when a different event takes this row's slot. + */ +export function MessageErrorBoundary({ + children, + resetKey, +}: { + children: ReactNode; + resetKey?: string; +}) { + return ( + + {children} + + ); +} diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 7d4bfda9..bf50b01a 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -125,6 +125,7 @@ import { useTheme } from '../../hooks/useTheme'; import { useRoomCreatorsTag } from '../../hooks/useRoomCreatorsTag'; import { usePowerLevelTags } from '../../hooks/usePowerLevelTags'; import { RoomTimelineTyping } from './RoomTimelineTyping'; +import { MessageErrorBoundary } from './MessageErrorBoundary'; const TimelineFloat = as<'div', css.TimelineFloatVariants>( ({ position, className, ...props }, ref) => ( @@ -2654,11 +2655,19 @@ export function RoomTimeline({ return ( {dayDividerJSX} - {eventJSX} + {eventJSX} ); } + if (eventJSX) { + return ( + + {eventJSX} + + ); + } + return eventJSX; }; diff --git a/src/app/features/room/ThreadDrawer.tsx b/src/app/features/room/ThreadDrawer.tsx index 3eb176bf..676d49aa 100644 --- a/src/app/features/room/ThreadDrawer.tsx +++ b/src/app/features/room/ThreadDrawer.tsx @@ -47,6 +47,7 @@ import { import { Image } from '../../components/media'; import { ImageViewer } from '../../components/image-viewer'; import { RenderMessageContent } from '../../components/RenderMessageContent'; +import { MessageErrorBoundary } from './MessageErrorBoundary'; import { RoomInput } from './RoomInput'; import { RoomInputPlaceholder } from './RoomInputPlaceholder'; import { @@ -1288,7 +1289,9 @@ export function ThreadDrawer({ return (
- {renderThreadEvent(rootEvent)} + + {renderThreadEvent(rootEvent)} + {/* Channel-style root↔replies divider / counter. Skipped in assistant mode, which renders a continuous ChatGPT-style transcript with no "N replies" affordance. */} {!assistantStyle && @@ -1364,7 +1367,11 @@ export function ThreadDrawer({ )} - {replies.map((reply) => renderThreadEvent(reply))} + {replies.map((reply) => ( + + {renderThreadEvent(reply)} + + ))} {botTyping && (
diff --git a/src/app/hooks/useIncomingRtcNotifications.ts b/src/app/hooks/useIncomingRtcNotifications.ts index b7d69b21..c78a3559 100644 --- a/src/app/hooks/useIncomingRtcNotifications.ts +++ b/src/app/hooks/useIncomingRtcNotifications.ts @@ -38,6 +38,7 @@ import { incomingCallsAtom } from '../state/incomingCalls'; import { getIncomingCallKey, getNotificationEventSendTs, + getRtcNotificationLifetime, isRtcNotificationExpired, RTC_NOTIFICATION_DEFAULT_LIFETIME, } from '../utils/rtcNotification'; @@ -253,8 +254,7 @@ export const useIncomingRtcNotifications = (): void => { }; const scheduleExpiry = (key: string, ev: MatrixEvent): ReturnType => { - const content = ev.getContent(); - const lifetime = content.lifetime ?? RTC_NOTIFICATION_DEFAULT_LIFETIME; + const lifetime = getRtcNotificationLifetime(ev); const expireAt = getNotificationEventSendTs(ev) + lifetime; const delay = Math.max(0, expireAt - Date.now()); return setTimeout(() => removeByKey(key), delay); @@ -415,7 +415,7 @@ export const useIncomingRtcNotifications = (): void => { // append-only, so we pass only the fields JS has reliable access to. const senderMember = room.getMember(sender); const callerName = senderMember?.rawDisplayName || room.name || sender || 'Vojo'; - const lifetime = content.lifetime ?? RTC_NOTIFICATION_DEFAULT_LIFETIME; + const lifetime = getRtcNotificationLifetime(ev); callForegroundService .upsertIncomingRing({ eventId: evId, diff --git a/src/app/hooks/usePowerLevels.ts b/src/app/hooks/usePowerLevels.ts index a39ccb19..625af650 100644 --- a/src/app/hooks/usePowerLevels.ts +++ b/src/app/hooks/usePowerLevels.ts @@ -58,7 +58,7 @@ const fillMissingPowers = (powerLevels: IPowerLevels): IPowerLevels => return draftPl; }); -const getPowersLevelFromMatrixEvent = (mEvent?: MatrixEvent): IPowerLevels => { +export const getPowersLevelFromMatrixEvent = (mEvent?: MatrixEvent): IPowerLevels => { const plContent = mEvent?.getContent(); const powerLevels = !plContent ? DEFAULT_POWER_LEVELS : fillMissingPowers(plContent); diff --git a/src/app/pages/Router.tsx b/src/app/pages/Router.tsx index 0b13144b..8ad5411c 100644 --- a/src/app/pages/Router.tsx +++ b/src/app/pages/Router.tsx @@ -122,7 +122,11 @@ function UserLinkRedirect() { const { userIdOrLocalPart } = useParams(); if (!userIdOrLocalPart) return ; - const raw = decodeURIComponent(userIdOrLocalPart); + // react-router already percent-decodes path params, so do NOT decode again: + // a second decodeURIComponent both throws (URIError) on a crafted link like + // `/u/%25` — white-screening the app via the router error boundary — and is + // semantically wrong (it un-escapes a further encoding layer). + const raw = userIdOrLocalPart; const mxid = isUserId(raw) && getMxIdServer(raw) ? raw : `@${raw}:${USER_LINK_HOST}`; if (!isUserId(mxid)) return ; diff --git a/src/app/plugins/call/CallWidgetDriver.ts b/src/app/plugins/call/CallWidgetDriver.ts index 6e85fa45..cbfa4982 100644 --- a/src/app/plugins/call/CallWidgetDriver.ts +++ b/src/app/plugins/call/CallWidgetDriver.ts @@ -58,10 +58,25 @@ import { downloadMedia, mxcUrlToHttp } from '../../utils/matrix'; const RTC_RING_LIFETIME_MAX_MS = 5 * 60 * 1000; const RTC_RING_MENTIONS_MAX_USERS = 64; +// Matrix event/user ids are bounded at 255 bytes by the spec. Bounding the +// fields here keeps a hostile widget from packing arbitrary data (e.g. media +// keys it received via io.element.call.encryption_keys) into a cleartext ring. +const RTC_RING_ID_MAX_LEN = 255; const isObject = (v: unknown): v is Record => typeof v === 'object' && v !== null && !Array.isArray(v); +// A ring's `event_id` must be a real event id: `$`-prefixed and length-bounded. +const isRingEventId = (v: unknown): v is string => + typeof v === 'string' && v.length > 1 && v.length <= RTC_RING_ID_MAX_LEN && v.startsWith('$'); + +// `m.mentions.user_ids` entries must look like mxids (`@local:server`) and stay +// within the spec length cap — not arbitrary unbounded strings. The anchors and +// `\S` classes forbid embedded whitespace/newlines so a hostile widget can't +// smuggle a multi-line blob in an otherwise mxid-shaped string. +const isRingUserId = (v: unknown): v is string => + typeof v === 'string' && v.length <= RTC_RING_ID_MAX_LEN && /^@[^:\s]+:\S+$/.test(v); + // Validate a widget-provided ring payload before we send it cleartext. Returns // the sanitized object on a happy path or `null` if any required field is // missing/malformed — caller then falls back to the encrypted send path so the @@ -72,7 +87,7 @@ function sanitizeRingContent(content: IContent): IContent | null { const relates = content['m.relates_to']; if (!isObject(relates)) return null; if (relates.rel_type !== 'm.reference') return null; - if (typeof relates.event_id !== 'string' || relates.event_id.length === 0) return null; + if (!isRingEventId(relates.event_id)) return null; const { sender_ts: senderTs, lifetime } = content; if (typeof senderTs !== 'number' || !Number.isFinite(senderTs) || senderTs <= 0) return null; @@ -103,7 +118,7 @@ function sanitizeRingContent(content: IContent): IContent | null { const sanitizedMentions: { user_ids?: string[]; room?: boolean } = {}; if (Array.isArray(mentions.user_ids)) { sanitizedMentions.user_ids = mentions.user_ids - .filter((u): u is string => typeof u === 'string') + .filter(isRingUserId) .slice(0, RTC_RING_MENTIONS_MAX_USERS); } if (typeof mentions.room === 'boolean') { diff --git a/src/app/utils/audioWaveform.ts b/src/app/utils/audioWaveform.ts index efa7411c..68250554 100644 --- a/src/app/utils/audioWaveform.ts +++ b/src/app/utils/audioWaveform.ts @@ -2,9 +2,20 @@ // normalized to 0..1. Handles both the on-wire MSC1767/Telegram integers // (0..1024) and a 0..1 float sender via max-detection (peak > 1 ⇒ divide by // peak). Used by the voice-note bubble and the recorder preview. +// The waveform arrives straight from hostile event content +// (`org.matrix.msc1767.audio.waveform`), so it may be ANY JSON value, not the +// `number[]` the type claims. Reject non-arrays and coerce non-finite samples +// to 0 — otherwise `.reduce` / `Math.abs` on a non-array (or a NaN sample) +// throws during render and, absent a per-message error boundary, white-screens +// the whole client (persistent DoS on the persisted event). +const sample = (v: unknown): number => { + const n = typeof v === 'number' ? v : Number(v); + return Number.isFinite(n) ? Math.abs(n) : 0; +}; + export const normalizeWaveform = (waveform: number[] | undefined, bars: number): number[] => { - if (!waveform || waveform.length === 0) return []; - const peak = waveform.reduce((m, v) => Math.max(m, Math.abs(v)), 0); + if (!Array.isArray(waveform) || waveform.length === 0) return []; + const peak = waveform.reduce((m, v) => Math.max(m, sample(v)), 0); const divisor = peak > 1 ? peak : 1; const out: number[] = []; const bucket = waveform.length / bars; @@ -13,7 +24,7 @@ export const normalizeWaveform = (waveform: number[] | undefined, bars: number): const end = Math.max(start + 1, Math.floor((i + 1) * bucket)); let localMax = 0; for (let j = start; j < end && j < waveform.length; j += 1) { - localMax = Math.max(localMax, Math.abs(waveform[j])); + localMax = Math.max(localMax, sample(waveform[j])); } out.push(Math.min(1, localMax / divisor)); } diff --git a/src/app/utils/room.ts b/src/app/utils/room.ts index de7dc07e..ab61f1d3 100644 --- a/src/app/utils/room.ts +++ b/src/app/utils/room.ts @@ -427,6 +427,7 @@ export const getDirectRoomAvatarUrl = ( }; export const trimReplyFromBody = (body: string): string => { + if (typeof body !== 'string') return ''; const match = body.match(/^> <.+?> .+\n(>.*\n)*?\n/m); if (!match) return body; return body.slice(match[0].length); diff --git a/src/app/utils/rtcNotification.ts b/src/app/utils/rtcNotification.ts index 58e32dd3..100b4c20 100644 --- a/src/app/utils/rtcNotification.ts +++ b/src/app/utils/rtcNotification.ts @@ -3,6 +3,25 @@ import { IRTCNotificationContent } from 'matrix-js-sdk/lib/matrixrtc/types'; export const RTC_NOTIFICATION_DEFAULT_LIFETIME = 30_000; +// Upper bound on a ring's lifetime. The value is fully attacker-controlled +// (raw peer event content), so without a cap a crafted `lifetime` (e.g. ~23 +// days, still under the 32-bit setTimeout ceiling) keeps the incoming-call +// strip + looping ringtone alive for the whole session, defeating the ~30s +// auto-expiry. Mirrors the 5-minute cap the send path already enforces +// (CallWidgetDriver `RTC_RING_LIFETIME_MAX_MS`). +export const RTC_NOTIFICATION_MAX_LIFETIME = 5 * 60 * 1000; + +// Resolve a ring's lifetime from event content, clamped to a sane range. A +// missing / non-finite / non-positive value falls back to the default; anything +// larger than the cap is clamped down. +export const getRtcNotificationLifetime = (ev: MatrixEvent): number => { + const { lifetime } = ev.getContent(); + if (typeof lifetime !== 'number' || !Number.isFinite(lifetime) || lifetime <= 0) { + return RTC_NOTIFICATION_DEFAULT_LIFETIME; + } + return Math.min(lifetime, RTC_NOTIFICATION_MAX_LIFETIME); +}; + export const getNotificationEventSendTs = (ev: MatrixEvent): number => { const content = ev.getContent(); const sendTs = content.sender_ts; @@ -12,8 +31,7 @@ export const getNotificationEventSendTs = (ev: MatrixEvent): number => { }; export const isRtcNotificationExpired = (ev: MatrixEvent, now = Date.now()): boolean => { - const content = ev.getContent(); - const lifetime = content.lifetime ?? RTC_NOTIFICATION_DEFAULT_LIFETIME; + const lifetime = getRtcNotificationLifetime(ev); return now - getNotificationEventSendTs(ev) > lifetime; }; diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index 8c7db9dd..a02e530c 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -140,8 +140,25 @@ export const logoutClient = async (mx: MatrixClient) => { } catch { // ignore if failed to logout } - await mx.clearStores(); + // Clear the credential FIRST. clearStores() deletes the crypto/sync + // IndexedDB stores, and its `onblocked` handler never resolves — a second + // open tab holding those stores open makes the await hang, which (in the old + // order) skipped localStorage.clear() and the redirect, leaving the access + // token on a shared device. Wiping localStorage up front fail-safes to + // "token gone" even if the store deletion blocks. window.localStorage.clear(); + // Best-effort crypto/sync store wipe, bounded so a blocked deletion can't + // hang logout — the credential is already gone by here. + try { + await Promise.race([ + mx.clearStores(), + new Promise((resolve) => { + window.setTimeout(resolve, 3000); + }), + ]); + } catch { + // ignore — credential already cleared above. + } // Navigate to root instead of reload(): on Android Capacitor the // WebViewLocalServer fails to SPA-fallback on URLs with `%3A` in path // segments (Matrix room/space ids), so reload of `///`