fix: sanitize notification HTML, restrict stream link scheme, fix replay/demo leaks (#542, #541, #550)#494
Open
Flegma wants to merge 1 commit into
Open
fix: sanitize notification HTML, restrict stream link scheme, fix replay/demo leaks (#542, #541, #550)#494Flegma wants to merge 1 commit into
Flegma wants to merge 1 commit into
Conversation
…replay + demo leaks - NotificationMessage: sanitize props.html with DOMPurify before parsing and rebuilding nodes, and render the SSR fallback as text instead of raw innerHTML. This was the only HTML sink in the app skipping DOMPurify, so a notification message embedding unescaped user text (team/player name) could reconstruct a javascript:/iframe/onerror XSS sink (#542). - StreamEmbed / MatchLiveStreams: reject non-http(s) stream links (z.url() accepts javascript:/data:). parseStreamLink and mountGenericIframe now scheme-guard before building the iframe (plus a sandbox attr on the generic embed), and openStream scheme-guards window.open and adds noopener (#541). - Replay3DLite: the pointerup/pointermove window listeners were anonymous, so cleanup could not remove them and their closures pinned the whole Three.js scene per remount. Named and removed on unmount (#550). - useDemoPlayback: the shared 1Hz poll timer, socket handler, and document visibilitychange listener were never torn down on unmount (notably dev attach mode, which skips stop()). Added ref-counted onScopeDispose cleanup that fires when the last consumer unmounts (#550).
Contributor
Author
|
Independent code review run on the diff: clean, no changes needed. The reviewer verified the useDemoPlayback ref-count is balanced across all call sites, the module-scoped visibilityHandler is correct, the DOMPurify config preserves the attributes the node rebuild needs, and (notably) the app runs |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Web security + leak fixes from the 2026-07 audit. Closes #542, #541, #550.
Changes
NotificationMessage HTML sanitization (#542)
NotificationMessage.vuesetcontainer.innerHTML = props.htmland rebuilt every node viah(tag, attrs, ...), copying all attributes verbatim with only anchorhrefs filtered. It was the only HTML sink in the app not passing throughDOMPurify(the news components both sanitize). Nowprops.htmlisDOMPurify.sanitized before parsing, and the SSR fallback renders as escaped text instead of rawinnerHTML. Strips scripts, event-handler attributes, andjavascript:/data:URIs so a notification embedding unescaped user text can't reconstruct an XSS sink.Stream link scheme restriction (#541)
z.string().url()acceptsjavascript:/data:URIs. A stored stream link flowed into an iframesrc(StreamEmbed.mountGenericIframe) andwindow.open(MatchLiveStreams.openStream) on public match pages.parseStreamLinknow rejects non-http(s) (and malformed) links, so they never become an embed.mountGenericIframescheme-guards defensively and adds asandboxattribute to the arbitrary third-party embed (keeps the player working, blocks top-window navigation).openStreamscheme-guardswindow.openand passesnoopener,noreferrer..refine()s the link to http(s).Resource leaks on unmount (#550)
Replay3DLite.vue: thepointerup/pointermovelisteners were added towindowas anonymous functions, socleanup()could not remove them and their closures pinned the entire Three.js scene (camera/controls/renderer) on every remount. Named them and addedremoveEventListenerin cleanup.useDemoPlayback.ts: the shared 1Hz poll timer, thedemo-session:statesocket handler, and the documentvisibilitychangelistener were never torn down on unmount (dev attach mode never routes throughstop()). Added ref-countedonScopeDisposecleanup that stops them when the last consumer unmounts, preserving the intentional shared-poll design.Testing
yarn build(nuxt build --standalone): green.Not included (deliberately deferred)
#535 (MatchOptions toggles double-fire on Space): on closer analysis for the fix, both the row
@clickand the switch's@update:model-valuepass the same target value tohandleChange(a set, not a relative toggle), so the double-fire is likely idempotent rather than a net no-op. The reported "net no-op" outcome needs confirming in a browser before changing the shared toggle-row pattern (used across MatchOptions and TournamentStageForm). Noted on #535; not fixed here to avoid a speculative UX change.