From 385602c19493feebd512927961dee8b432f03797 Mon Sep 17 00:00:00 2001 From: Timo <6156589+Shik3i@users.noreply.github.com> Date: Mon, 25 May 2026 02:11:54 +0200 Subject: [PATCH] fix(extension,server): resolve 12 audit findings (lobby cleanup, dedup race, protocol version, rate limiting, observer scope, and more) --- extension/background.js | 11 ++++++++--- extension/content.js | 2 +- extension/popup.js | 19 +++++++++++++++++-- server/index.js | 36 +++++++++++++++++++++--------------- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/extension/background.js b/extension/background.js index b68261a..777fc81 100644 --- a/extension/background.js +++ b/extension/background.js @@ -360,6 +360,9 @@ async function connect() { forceSyncAcks: [], forceSyncDeadline: null }); + + // Cancel any active episode lobby + clearEpisodeLobbyState(); if (currentRoom) { currentRoom.peers = []; @@ -412,6 +415,7 @@ function showNotification(senderName, action) { const label = action === 'play' ? 'started playback' : action === 'pause' ? 'paused playback' : action === 'seek' ? 'seeked the video' : + action === 'force_sync_prepare' ? 'started force sync' : action === 'force_sync_execute' ? 'synchronized everyone' : action; // Find username in current room if available @@ -954,6 +958,9 @@ function leaveOldRoomIfSwitching(newRoomId) { forceSyncAcks: [], forceSyncDeadline: null }); + + // Cancel any active episode lobby + clearEpisodeLobbyState(); } } @@ -1051,9 +1058,7 @@ async function handleAsyncMessage(message, sender, sendResponse) { } else if (message.type === 'GET_HISTORY') { sendResponse(history); } else if (message.type === 'GET_ROOM_LIST') { - if (socket && socket.readyState === WebSocket.OPEN) { - socket.send(`42${JSON.stringify([EVENTS.GET_ROOMS])}`); - } + emit(EVENTS.GET_ROOMS, {}); sendResponse({ status: 'ok' }); } else if (message.type === 'WEB_JOIN_REQUEST') { const { roomId, password, useCustomServer, serverUrl } = message; diff --git a/extension/content.js b/extension/content.js index c44190d..e0f30ad 100644 --- a/extension/content.js +++ b/extension/content.js @@ -519,7 +519,7 @@ observerTimeout = setTimeout(checkVideo, 1000 - (now - lastMutate)); } }); - observer.observe(document.body, { childList: true, subtree: true }); + observer.observe(document.documentElement, { childList: true, subtree: true }); // --- SHARED_HEARTBEAT_INJECT_START --- const HEARTBEAT_INTERVAL_VAL = 15000; diff --git a/extension/popup.js b/extension/popup.js index d7e1306..c7b5a50 100644 --- a/extension/popup.js +++ b/extension/popup.js @@ -231,6 +231,13 @@ function getVolumeIcon(volume, muted) { let activePeers = []; let interpolationInterval = null; +function stopInterpolation() { + if (interpolationInterval) { + clearInterval(interpolationInterval); + interpolationInterval = null; + } +} + function startInterpolation() { if (interpolationInterval) return; interpolationInterval = setInterval(() => { @@ -249,7 +256,11 @@ function startInterpolation() { function updatePeerList(peers) { if (!peers) return; activePeers = peers; - if (!interpolationInterval) startInterpolation(); + if (peers.length === 0) { + stopInterpolation(); + } else if (!interpolationInterval) { + startInterpolation(); + } // UI Throttle: Only re-render if the peer state actually changed (excluding time interpolation) const stateToHash = peers.map(p => ({ @@ -793,6 +804,10 @@ elements.forceSyncBtn.addEventListener('click', async () => { let targetTime = null; if (mode === 'jump-to-others') { + if (!localPeerId) { + showError('Identity not yet loaded. Wait a moment and try again.'); + return; + } const peers = status.peers || []; const otherTimes = peers .filter(p => typeof p === 'object' && p.peerId !== localPeerId && p.currentTime != null && !isNaN(p.currentTime)) @@ -888,7 +903,7 @@ async function refreshLogs() { logs.forEach(log => { const entry = document.createElement('div'); entry.className = `log-entry log-${log.type}`; - const timeStr = log.timestamp.split('T')[1].split('.')[0]; + const timeStr = log.timestamp?.split('T')?.[1]?.split('.')[0] || '?'; entry.textContent = `[${timeStr}] ${log.message}`; elements.logList.appendChild(entry); }); diff --git a/server/index.js b/server/index.js index 1a5d957..6032dfa 100644 --- a/server/index.js +++ b/server/index.js @@ -3,7 +3,7 @@ import { createServer } from 'http'; import { Server } from 'socket.io'; import bcrypt from 'bcryptjs'; import dotenv from 'dotenv'; -import { EVENTS, OFFICIAL_SERVER_TOKEN } from '../shared/constants.js'; +import { EVENTS, OFFICIAL_SERVER_TOKEN, PROTOCOL_VERSION } from '../shared/constants.js'; dotenv.config(); @@ -125,12 +125,8 @@ function checkEventRate(socketId) { * @param {string} socketId - The socket.id being removed. * @param {string} roomId - The room it belongs to. * @param {string} reason - Log label ('disconnect', 'leave', 'reaper', 'dedupe', 'room-switch'). - * @param {boolean} [emitLeave=true] - Set false when the socket.io room leave - * is handled by the caller (e.g. reaper calls - * socket.leave() before us, or dedupe calls - * oldSocket.leave() before disconnecting). */ -function removePeerFromRoom(socketId, roomId, reason, emitLeave = true) { +function removePeerFromRoom(socketId, roomId, reason) { const room = rooms.get(roomId); if (!room) return; @@ -225,7 +221,7 @@ io.on('connection', (socket) => { try { // Protocol check - if (protocolVersion !== '1.0.0') { + if (protocolVersion !== PROTOCOL_VERSION) { log('AUTH', `Protocol mismatch from ${peerId}: ${protocolVersion}`); socket.emit(EVENTS.ERROR, { message: 'Incompatible protocol version' }); return; @@ -279,18 +275,23 @@ io.on('connection', (socket) => { } // Peer Deduplication: Remove existing socket for the same peerId + // Snapshot stale SIDs first to avoid mutating the Map during iteration + const dedupeSids = []; for (const [sid, data] of room.peerData.entries()) { if (data.peerId === peerId && sid !== socket.id) { - const oldSocket = io.sockets.sockets.get(sid); - if (oldSocket) { - oldSocket.emit(EVENTS.ERROR, { message: 'Deduplication: Another session with this ID joined. Disconnecting...' }); - oldSocket.leave(roomId); - oldSocket.disconnect(true); - log('DEDUPE', `Kicked old session for peer ${peerId}`); - } - removePeerFromRoom(sid, roomId, 'dedupe'); + dedupeSids.push(sid); } } + for (const sid of dedupeSids) { + const oldSocket = io.sockets.sockets.get(sid); + if (oldSocket) { + oldSocket.emit(EVENTS.ERROR, { message: 'Deduplication: Another session with this ID joined. Disconnecting...' }); + oldSocket.leave(roomId); + oldSocket.disconnect(true); + log('DEDUPE', `Kicked old session for peer ${peerId}`); + } + removePeerFromRoom(sid, roomId, 'dedupe'); + } } socket.join(roomId); @@ -404,6 +405,11 @@ io.on('connection', (socket) => { }); socket.on(EVENTS.EVENT_ACK, (data) => { + if (!checkEventRate(socket.id)) { + log('SECURITY', `Event rate limit exceeded for socket (ACK): ${socket.id}`); + socket.disconnect(true); + return; + } if (!data || typeof data !== 'object') return; if (typeof data.targetId !== 'string') return; if (data.actionTimestamp !== undefined && (typeof data.actionTimestamp !== 'number' || !Number.isFinite(data.actionTimestamp))) return;