From f7829bbebbae7d2223aa70773c77f35c538f91ed Mon Sep 17 00:00:00 2001
From: MacBook <macbook@shik3i.net>
Date: Mon, 4 May 2026 05:19:18 +0200
Subject: [PATCH] security: harden server relay + documentation audit

Server Security (S-1 through S-8):
- S-1: Type-check and clamp peerId, protocolVersion, password
- S-2: Validate numeric/boolean/enum fields in relay peerData
- S-3: Construct explicit relay payload (stop spreading raw data)
- S-4: Type-check targetId and actionTimestamp in EVENT_ACK
- S-5: Restrict room IDs to [a-zA-Z0-9-] only
- S-7: Add eventCounts periodic cleanup alongside connectionCounts
- S-8: Guard version parsing against NaN bypass

Documentation (P-1, R-1 through R-6):
- P-1: Fix PRIVACY.md typo, document all in-memory data maps
- R-1/R-5: Fix stale sync-constants.bat references in shared/
- R-2: Fix stale lastTargetState ref in ARCHITECTURE.md
- R-3: Extension README title reflects cross-browser support
- R-6: Document content injection markers in scripts/README.md
---
 PRIVACY.md           |  6 ++--
 docs/ARCHITECTURE.md |  2 +-
 extension/README.md  |  4 +--
 scripts/README.md    | 14 ++++++++-
 server/index.js      | 71 +++++++++++++++++++++++++++++++++-----------
 shared/blacklist.js  |  2 +-
 shared/constants.js  |  2 +-
 7 files changed, 75 insertions(+), 26 deletions(-)

diff --git a/PRIVACY.md b/PRIVACY.md
index 64487f8..4547c86 100644
--- a/PRIVACY.md
+++ b/PRIVACY.md
@@ -4,13 +4,15 @@ KoalaSync is designed with a **Security-First & Volatile** architecture. This me
 
 ## 1. Data Processing (In-Memory Only)
 KoalaSync does not use a database. All active session data exists only in the server's RAM and is purged immediately when no longer needed.
-- **Session Data**: To synchronize playback, the server must temporarily hold your `peerId`, `username`, and the `title` of the video you are watching. This is deleted as soon as you leave the room.
+- **Session Data**: To synchronize playback, the server must temporarily hold your `peerId`, `username`, and the `title` of the video you are watching. Additionally, playback metadata (`mediaTitle`, `playbackState`, `currentTime`, `volume`, `muted`) is held per peer for the duration of the session. All of this is deleted as soon as you leave the room.
 - **Room Passwords**: If you set a room password, it is stored only as a secure **bcrypt hash** in RAM. The server never sees or stores your plaintext password.
+- **Routing Maps**: The server maintains ephemeral lookup tables (`socketToRoom`, `peerToSocket`) to route messages between peers. These contain only transport identifiers and are purged on disconnect.
 
 ## 2. Security & Rate Limiting
 To prevent abuse and brute-force attacks, the following data is processed:
 - **Brute-Force Protection**: If multiple failed password attempts are detected, the server stores the `IP address` and `Room ID` in a temporary RAM-based lockout list for a maximum of 15 minutes.
 - **Connection Rate Limiting**: IP addresses are tracked for 60 seconds to prevent connection-flooding (DoS) attacks.
+- **Event Rate Limiting**: Per-socket event counters are tracked for 10-second windows to prevent event-spamming. These are keyed by ephemeral socket IDs and cleaned up periodically.
 - **Console Logging**: The official relay server (`sync.shik3i.net`) outputs connection events (including IP addresses) to the server console for real-time monitoring. These logs are ephemeral and are not archived, sold, or linked to any persistent user identity.
 
 ## 3. Extension Permissions
@@ -21,7 +23,7 @@ The browser extension requires the following permissions:
 
 ## 4. Zero Third-Party Requests
 KoalaSync is completely self-contained:
-- **No CDNs or CDNs**: All scripts and styles are self-hosted.
+- **No CDNs or External Libraries**: All scripts and styles are self-hosted.
 - **No Analytics**: We do not use Google Analytics, tracking pixels, or any third-party telemetry.
 - **No External Fonts**: We use system font stacks to prevent tracking via font services.
 
diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md
index 9d010cb..8c0feec 100644
--- a/docs/ARCHITECTURE.md
+++ b/docs/ARCHITECTURE.md
@@ -16,7 +16,7 @@ This document describes the communication flows and internal logic of the KoalaS
 ## 2. Media Event Synchronization
 When a user interacts with a video:
 1. **Detection**: `content.js` listens to native events (`play`, `pause`, `seeked`) on the `<video>` element.
-2. **Prevention of Loops**: Uses `lastTargetState` to distinguish between user actions and programmatic actions triggered by the extension.
+2. **Prevention of Loops**: Uses an `expectedEvents` Set to distinguish between user actions and programmatic actions. Expected events are consumed on match and expire via timeout.
 3. **Reporting**: `content.js` sends a `CONTENT_EVENT` to `background.js`.
 4. **Relay**: The Server forwards the event to all other peers in the room.
 5. **Execution**: Remote peers receive the command and call `video.play()`, `video.pause()`, or `video.currentTime = targetTime`.
diff --git a/extension/README.md b/extension/README.md
index 2b48d6e..1f29a7f 100644
--- a/extension/README.md
+++ b/extension/README.md
@@ -1,6 +1,6 @@
-# KoalaSync Chrome Extension
+# KoalaSync Browser Extension
 
-A Manifest V3 Chrome Extension for synchronized video playback across any website.
+A Manifest V3 Browser Extension (Chrome & Firefox) for synchronized video playback across any website.
 
 ## Key Features
 - **Manifest V3**: Optimized Service Worker architecture with session persistence.
diff --git a/scripts/README.md b/scripts/README.md
index 83dd355..4fdbf11 100644
--- a/scripts/README.md
+++ b/scripts/README.md
@@ -7,7 +7,8 @@ This directory contains utility scripts for the KoalaSync development workflow.
 The primary build tool for KoalaSync. This Node.js script automates two critical tasks:
 
 1.  **Protocol Synchronization**: Copies the "Single Source of Truth" constants (`shared/constants.js`) and the domain blacklist (`shared/blacklist.js`) from the root `/shared` directory into the `extension/shared/` directory.
-2.  **Artifact Generation**: Compiles the extension into browser-specific bundles for Chrome and Firefox, located in the `dist/` directory.
+2.  **Content Script Injection**: Injects protocol constants directly into `content.js` using marker-based replacement. This is necessary because `content.js` executes synchronously and cannot use ES module imports.
+3.  **Artifact Generation**: Compiles the extension into browser-specific bundles for Chrome and Firefox, located in the `dist/` directory.
 
 ### Usage
 
@@ -19,3 +20,14 @@ node scripts/build-extension.js
 
 ### Why this script exists
 KoalaSync uses **Vanilla JS** in the extension to maintain zero runtime dependencies and maximum privacy. Since we don't use a bundler (like Webpack or Vite) inside the extension, this script serves as our lightweight "pre-build" step to ensure that the protocol constants remain synchronized between the extension and the relay server.
+
+### Content Injection Markers
+
+The build script uses marker comments in `content.js` to locate and replace constant blocks:
+
+| Marker Pair | Injected Value | Source |
+|:---|:---|:---|
+| `SHARED_EVENTS_INJECT_START` / `END` | The full `EVENTS` object | `shared/constants.js` |
+| `SHARED_HEARTBEAT_INJECT_START` / `END` | `HEARTBEAT_INTERVAL` value | `shared/constants.js` |
+
+> **⚠️ Do NOT remove or modify these marker comments in `content.js`.** They are required for the build script to function. If the markers are missing, the build will fail with a clear error message.
diff --git a/server/index.js b/server/index.js
index c2fcbd4..555c969 100644
--- a/server/index.js
+++ b/server/index.js
@@ -85,7 +85,7 @@ setInterval(() => {
 
 const eventCounts = new Map(); // socketId -> { count, resetTime }
 
-// Clean up connection counts to prevent memory leak
+// Clean up connection counts and event counts to prevent memory leak
 setInterval(() => {
     const now = Date.now();
     for (const [ip, entry] of connectionCounts.entries()) {
@@ -93,6 +93,11 @@ setInterval(() => {
             connectionCounts.delete(ip);
         }
     }
+    for (const [socketId, entry] of eventCounts.entries()) {
+        if (now > entry.resetTime) {
+            eventCounts.delete(socketId);
+        }
+    }
 }, 60000);
 
 function checkConnectionRate(ip) {
@@ -182,6 +187,12 @@ io.on('connection', (socket) => {
     if (clientVersion) {
         const [cMaj, cMin, cPatch] = clientVersion.split('.').map(Number);
         const [mMaj, mMin, mPatch] = MIN_VERSION.split('.').map(Number);
+        if (isNaN(cMaj) || isNaN(cMin) || isNaN(cPatch)) {
+            log('AUTH', `Invalid version format (${clientVersion}) from ${clientIp}`);
+            socket.emit(EVENTS.ERROR, { message: 'Invalid version format' });
+            socket.disconnect(true);
+            return;
+        }
         const tooOld = cMaj < mMaj || (cMaj === mMaj && cMin < mMin) || (cMaj === mMaj && cMin === mMin && cPatch < mPatch);
         if (tooOld) {
             log('AUTH', `Version too old (${clientVersion}) from ${clientIp}`);
@@ -200,15 +211,17 @@ io.on('connection', (socket) => {
             return;
         }
         if (!payload || typeof payload.roomId !== 'string') return;
-        const { password, peerId, protocolVersion } = payload;
 
-        // --- M-2: Sanitize and clamp all string fields ---
-        const roomId   = String(payload.roomId   || '').substring(0, 64);
+        // --- S-1 & S-5: Sanitize and clamp all incoming fields ---
+        const password        = typeof payload.password === 'string' ? payload.password.substring(0, 128) : null;
+        const peerId          = typeof payload.peerId === 'string' ? payload.peerId.substring(0, 16) : null;
+        const protocolVersion = typeof payload.protocolVersion === 'string' ? payload.protocolVersion.substring(0, 16) : null;
+        const roomId   = String(payload.roomId || '').replace(/[^a-zA-Z0-9\-]/g, '').substring(0, 64);
         const username = typeof payload.username  === 'string' ? payload.username.substring(0, 30)  : null;
         const tabTitle = typeof payload.tabTitle  === 'string' ? payload.tabTitle.substring(0, 100) : null;
         const mediaTitle = typeof payload.mediaTitle === 'string' ? payload.mediaTitle.substring(0, 100) : null;
 
-        if (!roomId) return; // Guard: empty after sanitization
+        if (!roomId || !peerId) return; // Guard: empty or invalid after sanitization
 
         try {
             // Protocol check
@@ -329,24 +342,45 @@ io.on('connection', (socket) => {
                 if (room) {
                     room.lastActivity = Date.now();
                     
-                    // Update peer metadata and lastSeen
-                    // Sanitize mutable string fields to enforce the same length
-                    // limits as JOIN_ROOM — the relay path is otherwise unbounded.
-                    const clamp = (val, max) => typeof val === 'string' ? val.substring(0, max) : val;
+                    // --- S-2 & S-3: Sanitize ALL relay fields (strings, numbers, booleans) ---
+                    const clamp    = (val, max) => typeof val === 'string' ? val.substring(0, max) : undefined;
+                    const clampNum = (val, min, max) => typeof val === 'number' && Number.isFinite(val) ? Math.max(min, Math.min(max, val)) : undefined;
+                    const validState = (val) => (val === 'playing' || val === 'paused') ? val : undefined;
+                    const validBool  = (val) => typeof val === 'boolean' ? val : undefined;
+
                     const existing = room.peerData.get(socket.id) || { peerId: mapping.peerId };
                     room.peerData.set(socket.id, { 
                         ...existing,
-                        username:      data.username      !== undefined ? clamp(data.username, 30)   : existing.username,
-                        tabTitle:      data.tabTitle      !== undefined ? clamp(data.tabTitle, 100)  : existing.tabTitle,
-                        mediaTitle:    data.mediaTitle    !== undefined ? clamp(data.mediaTitle, 100) : existing.mediaTitle,
-                        playbackState: data.playbackState !== undefined ? data.playbackState          : existing.playbackState,
-                        currentTime:   data.currentTime   !== undefined ? data.currentTime            : existing.currentTime,
-                        volume:        data.volume        !== undefined ? data.volume                 : existing.volume,
-                        muted:         data.muted         !== undefined ? data.muted                  : existing.muted,
+                        username:      data.username      !== undefined ? (clamp(data.username, 30)   ?? existing.username)      : existing.username,
+                        tabTitle:      data.tabTitle      !== undefined ? (clamp(data.tabTitle, 100)  ?? existing.tabTitle)      : existing.tabTitle,
+                        mediaTitle:    data.mediaTitle    !== undefined ? (clamp(data.mediaTitle, 100) ?? existing.mediaTitle)   : existing.mediaTitle,
+                        playbackState: data.playbackState !== undefined ? (validState(data.playbackState) ?? existing.playbackState) : existing.playbackState,
+                        currentTime:   data.currentTime   !== undefined ? (clampNum(data.currentTime, 0, 86400) ?? existing.currentTime)   : existing.currentTime,
+                        volume:        data.volume        !== undefined ? (clampNum(data.volume, 0, 1) ?? existing.volume)                 : existing.volume,
+                        muted:         data.muted         !== undefined ? (validBool(data.muted) ?? existing.muted)                       : existing.muted,
                         lastSeen: Date.now()
                     });
 
-                    socket.to(mapping.roomId).emit(eventName, { ...data, senderId: mapping.peerId });
+                    // --- S-3: Construct clean relay payload — never forward raw client data ---
+                    const relayPayload = {
+                        senderId:        mapping.peerId,
+                        currentTime:     clampNum(data.currentTime, 0, 86400),
+                        targetTime:      clampNum(data.targetTime, 0, 86400),
+                        playbackState:   validState(data.playbackState),
+                        username:        clamp(data.username, 30),
+                        tabTitle:        clamp(data.tabTitle, 100),
+                        mediaTitle:      clamp(data.mediaTitle, 100),
+                        volume:          clampNum(data.volume, 0, 1),
+                        muted:           validBool(data.muted),
+                        peerId:          typeof data.peerId === 'string' ? data.peerId.substring(0, 16) : undefined,
+                        status:          typeof data.status === 'string' ? data.status.substring(0, 16) : undefined,
+                        expectedTitle:   clamp(data.expectedTitle, 100),
+                        title:           clamp(data.title, 100),
+                        actionTimestamp:  clampNum(data.actionTimestamp, 0, Number.MAX_SAFE_INTEGER),
+                    };
+                    // Strip undefined keys for clean wire format
+                    Object.keys(relayPayload).forEach(k => relayPayload[k] === undefined && delete relayPayload[k]);
+                    socket.to(mapping.roomId).emit(eventName, relayPayload);
                 }
             }
         });
@@ -371,7 +405,8 @@ io.on('connection', (socket) => {
 
         socket.on(EVENTS.EVENT_ACK, (data) => {
             if (!data || typeof data !== 'object') return;
-            if (!data.targetId) return;
+            if (typeof data.targetId !== 'string') return;
+            if (data.actionTimestamp !== undefined && (typeof data.actionTimestamp !== 'number' || !Number.isFinite(data.actionTimestamp))) return;
             
             const senderMapping = socketToRoom.get(socket.id);
             const targetSocketId = peerToSocket.get(data.targetId);
diff --git a/shared/blacklist.js b/shared/blacklist.js
index b41c23b..852e90d 100644
--- a/shared/blacklist.js
+++ b/shared/blacklist.js
@@ -2,7 +2,7 @@
  * blacklist.js
  * 
  * ⚠️ WARNING: This is the SINGLE SOURCE OF TRUTH.
- * If you edit this file, you MUST run /scripts/sync-constants.bat 
+ * If you edit this file, you MUST run: node scripts/build-extension.js
  * to propagate changes to the extension and relay server.
  * 
  * Domains to be filtered out from the tab selection dropdown to reduce "noise".
diff --git a/shared/constants.js b/shared/constants.js
index bc1a1e8..565d699 100644
--- a/shared/constants.js
+++ b/shared/constants.js
@@ -2,7 +2,7 @@
  * KoalaSync Shared Constants & Protocol Definitions
  * 
  * ⚠️ WARNING: This is the SINGLE SOURCE OF TRUTH.
- * If you edit this file, you MUST run /scripts/sync-constants.bat 
+ * If you edit this file, you MUST run: node scripts/build-extension.js
  * to propagate changes to the extension and relay server.
  */