mirror of
https://github.com/Shik3i/KoalaSync.git
synced 2026-07-26 12:08:15 +00:00
fix(popup): render lobby peer names as text, not markup
Peer usernames are remote-controlled and were interpolated into an innerHTML string in updateLobbyUI. The server only truncates them to 30 chars, so a peer could inject markup into everyone else's popup: enough to load a remote image (leaking viewer IPs) or spoof readiness badges. Inline handlers were already blocked by the MV3 default CSP. Build the peer items with the DOM API, matching the pattern the sibling peer list already uses. Add the two checks that would have caught this before upload: - eslint no-unsanitized, which reproduces the AMO warning at lint time - addons-linter on the built XPI in verify-release, with --warnings-as-errors since it exits 0 on warnings and AMO rejects them Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -4,6 +4,16 @@ All notable changes to the KoalaSync browser extension and relay server.
|
||||
|
||||
---
|
||||
|
||||
## [v2.6.1] — 2026-07-15
|
||||
|
||||
### Fixed
|
||||
- **Extension: Episode Lobby peer list** — Peer names in the lobby are now rendered as text instead of markup. A peer could previously put HTML in their username and have it rendered in everyone else's popup, which allowed loading remote images (leaking viewer IP addresses) and spoofing the readiness badges. Scripts were already blocked by the extension's content security policy.
|
||||
|
||||
### Changed
|
||||
- **Release checks: AMO validation** — `npm run verify` now runs Mozilla's `addons-linter` against the built Firefox artifact with `--warnings-as-errors`, and ESLint enforces `no-unsanitized`, so upload-blocking issues surface locally instead of at submission time.
|
||||
|
||||
---
|
||||
|
||||
## [v2.6.0] — 2026-07-15
|
||||
|
||||
### Added
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import noUnsanitized from "eslint-plugin-no-unsanitized";
|
||||
|
||||
export default [
|
||||
{
|
||||
ignores: ["coverage/**", "dist/**", "node_modules/**", "scratch/**", "website/www/**"]
|
||||
@@ -45,7 +47,14 @@ export default [
|
||||
process: "readonly",
|
||||
}
|
||||
},
|
||||
plugins: {
|
||||
"no-unsanitized": noUnsanitized
|
||||
},
|
||||
rules: {
|
||||
// Mirrors the AMO validator's "Unsafe assignment to innerHTML" check, so a
|
||||
// rejection at upload time surfaces here instead.
|
||||
"no-unsanitized/property": "error",
|
||||
"no-unsanitized/method": "error",
|
||||
"no-undef": "error",
|
||||
"no-unused-vars": ["error", { "argsIgnorePattern": "^_", "varsIgnorePattern": "^_", "caughtErrorsIgnorePattern": "^_" }],
|
||||
"no-unreachable": "error",
|
||||
|
||||
+20
-5
@@ -2603,9 +2603,10 @@ function updateLobbyUI(lobby, peers) {
|
||||
elements.episodeLobbyCard.style.display = 'block';
|
||||
elements.lobbyTitle.textContent = getMessage('LOBBY_WAITING_FOR', { title: lobby.expectedTitle });
|
||||
|
||||
// Build peer readiness list
|
||||
// Build peer readiness list. Peer usernames are remote-controlled input, so
|
||||
// every node is built with the DOM API and filled via textContent.
|
||||
const readySet = new Set(lobby.readyPeers || []);
|
||||
const peerHtmls = [];
|
||||
const peerItems = [];
|
||||
|
||||
if (peers && peers.length > 0) {
|
||||
peers.forEach(p => {
|
||||
@@ -2616,12 +2617,26 @@ function updateLobbyUI(lobby, peers) {
|
||||
const icon = isReady ? '✅' : '⏳';
|
||||
const label = isReady ? getMessage('LABEL_LOBBY_PEER_READY') : getMessage('LABEL_LOBBY_PEER_LOADING');
|
||||
const badgeClass = isReady ? 'badge-ready' : 'badge-loading';
|
||||
peerHtmls.push(`<span class="lobby-peer-item">${icon} ${avatar} ${pName} <span class="badge ${badgeClass}">${label}</span></span>`);
|
||||
|
||||
const item = document.createElement('span');
|
||||
item.className = 'lobby-peer-item';
|
||||
item.appendChild(document.createTextNode(`${icon} ${avatar} ${pName} `));
|
||||
|
||||
const badge = document.createElement('span');
|
||||
badge.className = `badge ${badgeClass}`;
|
||||
badge.textContent = label;
|
||||
item.appendChild(badge);
|
||||
|
||||
peerItems.push(item);
|
||||
});
|
||||
}
|
||||
|
||||
if (peerHtmls.length > 0 && elements.lobbyPeerStatus) {
|
||||
elements.lobbyPeerStatus.innerHTML = peerHtmls.join(' ');
|
||||
if (peerItems.length > 0 && elements.lobbyPeerStatus) {
|
||||
elements.lobbyPeerStatus.textContent = '';
|
||||
peerItems.forEach((item, index) => {
|
||||
if (index > 0) elements.lobbyPeerStatus.appendChild(document.createTextNode(' '));
|
||||
elements.lobbyPeerStatus.appendChild(item);
|
||||
});
|
||||
} else if (elements.lobbyPeerStatus) {
|
||||
elements.lobbyPeerStatus.textContent = getMessage('LOBBY_WAITING_PEERS');
|
||||
}
|
||||
|
||||
Generated
+1500
-13
File diff suppressed because it is too large
Load Diff
@@ -17,9 +17,11 @@
|
||||
"devDependencies": {
|
||||
"@playwright/test": "^1.61.1",
|
||||
"@vitest/coverage-v8": "^4.1.9",
|
||||
"addons-linter": "^10.9.0",
|
||||
"archiver": "^7.0.1",
|
||||
"esbuild": "^0.28.1",
|
||||
"eslint": "^10.4.0",
|
||||
"eslint-plugin-no-unsanitized": "^4.1.5",
|
||||
"fs-extra": "^11.2.0",
|
||||
"htmlnano": "^3.4.0",
|
||||
"sharp": "^0.34.5",
|
||||
|
||||
@@ -33,6 +33,10 @@ const checks = [
|
||||
['root production audit', 'npm', ['audit', '--omit=dev']],
|
||||
['server production audit', 'npm', ['audit', '--omit=dev'], { cwd: path.join(repoRoot, 'server') }],
|
||||
['extension build', 'npm', ['run', 'build:extension']],
|
||||
// Must run after the build: validates the exact artifact uploaded to AMO.
|
||||
// --warnings-as-errors is required: addons-linter exits 0 on warnings, and AMO
|
||||
// rejects on them, so without it this check would pass a rejectable build.
|
||||
['AMO validation (firefox)', 'npx', ['addons-linter', '--warnings-as-errors', 'dist/koalasync-firefox.zip']],
|
||||
['website build', 'node', ['website/build.cjs']]
|
||||
];
|
||||
|
||||
|
||||
@@ -599,6 +599,7 @@ document.addEventListener('DOMContentLoaded', () => {
|
||||
}
|
||||
const isDE = document.documentElement.classList.contains('lang-de');
|
||||
title.textContent = isDE ? 'Erfolgreich!' : 'Success!';
|
||||
// eslint-disable-next-line no-unsanitized/property -- both branches are hardcoded literals, no dynamic input
|
||||
desc.innerHTML = isDE
|
||||
? 'Verbunden! <br><span style="color:var(--accent); font-weight:bold;">Wähle jetzt einen Video-Tab in der Erweiterung aus.</span>'
|
||||
: 'Connected! <br><span style="color:var(--accent); font-weight:bold;">Now select a video tab in the extension.</span>';
|
||||
@@ -616,6 +617,7 @@ document.addEventListener('DOMContentLoaded', () => {
|
||||
setTimeout(updateCountdown, 1000);
|
||||
|
||||
const closeLabel = isDE ? 'TAB JETZT SCHLIESSEN' : 'CLOSE TAB NOW';
|
||||
// eslint-disable-next-line no-unsanitized/property -- closeLabel is a hardcoded literal, no dynamic input
|
||||
actions.innerHTML = `
|
||||
<div class="join-card-actions">
|
||||
<button class="btn btn-success" onclick="window.close()">${closeLabel}</button>
|
||||
@@ -634,6 +636,7 @@ document.addEventListener('DOMContentLoaded', () => {
|
||||
title.textContent = isDE ? 'Fehler' : 'Error';
|
||||
desc.textContent = isDE ? `Beitritt fehlgeschlagen: ${message}` : `Join failed: ${message}`;
|
||||
const retryLabel = isDE ? 'ERNEUT VERSUCHEN' : 'TRY AGAIN';
|
||||
// eslint-disable-next-line no-unsanitized/property -- retryLabel is a hardcoded literal, no dynamic input
|
||||
actions.innerHTML = `
|
||||
<div class="join-card-actions">
|
||||
<button class="btn btn-primary" onclick="location.reload()">${retryLabel}</button>
|
||||
@@ -1366,10 +1369,12 @@ document.addEventListener('DOMContentLoaded', () => {
|
||||
const isDE = document.documentElement.classList.contains('lang-de');
|
||||
const originalHTML = copyBtn.innerHTML;
|
||||
|
||||
// eslint-disable-next-line no-unsanitized/property -- both branches are hardcoded literals, no dynamic input
|
||||
copyBtn.innerHTML = isDE ? '✅ Kopiert!' : '✅ Copied!';
|
||||
copyBtn.disabled = true;
|
||||
|
||||
setTimeout(() => {
|
||||
// eslint-disable-next-line no-unsanitized/property -- restores markup captured from this same button above
|
||||
copyBtn.innerHTML = originalHTML;
|
||||
copyBtn.disabled = false;
|
||||
}, 2000);
|
||||
|
||||
Reference in New Issue
Block a user