From a664a8bbce42ebfb03e0255c3a9de757c095e503 Mon Sep 17 00:00:00 2001 From: jwinterm Date: Sun, 31 May 2026 09:40:34 -0400 Subject: [PATCH] security: escape XSS sinks client-side and add CSP + security headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client trusted the server to pre-escape chat content and rendered user-supplied fields via .html()/innerHTML — one server regression away from stored XSS, and the admin panel injected user-controlled payout addresses into its authenticated DOM. - Add a single shared escapeHtml helper (escapes all five HTML-significant chars including both quotes); leaderboard now delegates to it. - Escape chat username + message in onChatBroadcast/onChatHistory. - Escape user-controlled fields in the admin panel (payout address, socket id, player name, status) and make its local escapeHtml quote-safe so values in title="..." can't break out. - Add CSP plus X-Content-Type-Options, X-Frame-Options and Referrer-Policy headers via middleware. connect-src 'self' keeps Socket.IO working; removing 'unsafe-inline' (nonces) is tracked as a later cleanup. Full suite: 13 suites / 119 tests. --- docs/REMEDIATION_PLAN.md | 2 +- html/admin.html | 26 ++++++++++++++++---------- html/index.html | 1 + html/js/core/escapeHtml.js | 18 ++++++++++++++++++ html/js/network/socketHandlers.js | 6 ++++-- html/js/ui/leaderboard.js | 11 +++++++---- src/index.js | 24 ++++++++++++++++++++++++ 7 files changed, 71 insertions(+), 17 deletions(-) create mode 100644 html/js/core/escapeHtml.js diff --git a/docs/REMEDIATION_PLAN.md b/docs/REMEDIATION_PLAN.md index 5b51cfd..0bdb96d 100644 --- a/docs/REMEDIATION_PLAN.md +++ b/docs/REMEDIATION_PLAN.md @@ -22,7 +22,7 @@ Key: `S` ≤ half day · `M` ~1–3 days · `L` ~1 week+ · 🔴 launch-blocker - [x] **0.3 🔴 M — Fix batch-payout `tx_hash` collision + double-pay.** ✅ Migration 015 drops the payouts `tx_hash` unique index (batches legitimately share a tx_hash) and extends the one-per-game guard to cover `processing`. Batch completion now happens in ONE transaction via `id = ANY` (no row stranded mid-batch); BIGINT stat math (no float). Ambiguous batch failures → `needs_review` (retry service won't re-send) + operator alert. New `batchPayout.test.js` covers both paths. - [x] **0.4 🔴 S — Harden identity transport.** ✅ Client sends `anon_token` in handshake `auth` (not the URL query → no proxy/referer/access-log leakage); server reads `auth` first with a query-string fallback for older clients. Token is now **rotated on every resume** (`rotateToken`), so a leaked token is invalidated when the real owner reconnects; client already persists the returned token. Rotation + old-token-replay-rejection test added to `security.test.js`. - [x] **0.5 🟠 S — Single-game disconnect recovery.** ✅ `recoverPendingPayments` now also recovers confirmed-but-unconsumed `single_game` payments (no `games.payment_id` link) by granting the equivalent credits, idempotently (per-payment reason key, row-locked). Disconnect preservation made deterministic: the stable `dbUserId` is **stamped on the game at creation**, so suspend/restore never needs a fragile disconnect-time DB lookup — every game (paid or free) is reconnectable whenever a session exists; the drop-branch is now only the genuine no-session edge. New `singleGameRecovery.test.js` (grant / consumed / idempotent). -- [ ] **0.6 🟠 S — Client + admin XSS and CSP.** Escape chat/admin fields (`html/js/network/socketHandlers.js:291,323`; `html/admin.html`); add CSP headers. +- [x] **0.6 🟠 S — Client + admin XSS and CSP.** ✅ Single shared `escapeHtml` helper (`html/js/core/escapeHtml.js`, escapes all five chars incl. both quotes); leaderboard delegates to it. Chat sinks (`onChatBroadcast`/`onChatHistory`) now escape username + message. Admin panel escapes user-controlled fields (payout address, socket id, player name, status) and its local `escapeHtml` now escapes quotes (safe in `title="…"`). CSP + `X-Content-Type-Options`/`X-Frame-Options`/`Referrer-Policy` headers added via middleware in `index.js` (`connect-src 'self'` keeps Socket.IO working; nonce-based tightening tracked as Phase 4.4). ## Phase 1 — Money correctness hardening (🟠) - [ ] **1.1 M — `src/money/atomic.js` BigInt module** as the only money type; remove `Number()` casts in `gameModeManager.js` and float `reduce` in `walletRPCService.js:314`. diff --git a/html/admin.html b/html/admin.html index 0c5d425..fe023db 100644 --- a/html/admin.html +++ b/html/admin.html @@ -738,9 +738,9 @@ ${p.id} ${(p.amount / atomicDivisor).toFixed(6)} - ${p.address ? p.address.substring(0, 12) + '...' : '-'} - ${p.status} - ${p.txHash ? p.txHash.substring(0, 16) + '...' : '-'} + ${p.address ? escapeHtml(p.address.substring(0, 12)) + '...' : '-'} + ${escapeHtml(p.status)} + ${p.txHash ? escapeHtml(p.txHash.substring(0, 16)) + '...' : '-'} ${new Date(p.createdAt).toLocaleString()} ${p.status === 'failed' || p.status === 'permanently_failed' ? @@ -821,8 +821,8 @@ tbody.innerHTML = data.users.map(u => ` ${u.id} - ${u.socketId ? u.socketId.substring(0, 15) + '...' : '-'} - ${u.payoutAddress || '-'} + ${u.socketId ? escapeHtml(u.socketId.substring(0, 15)) + '...' : '-'} + ${u.payoutAddress ? escapeHtml(u.payoutAddress) : '-'} ${u.credits || 0} ${u.gamesPlayed || 0} ${u.gamesWon || 0} @@ -879,9 +879,9 @@ ${m.id} ${formatTimeAgo(new Date(m.timestamp))} - ${m.playerId || m.username || '-'} + ${escapeHtml(m.playerId || m.username || '-')} ${escapeHtml(m.message)} - ${m.userId || '-'} + ${escapeHtml(m.userId || '-')} ${isDeleted ? 'Deleted' : ''} ${isBanned ? 'Banned' : ''} @@ -978,9 +978,15 @@ } function escapeHtml(text) { - const div = document.createElement('div'); - div.textContent = text; - return div.innerHTML; + // Escape all five HTML-significant chars (including quotes) so the result is + // safe in both element text and quoted attribute values (e.g. title="..."). + if (text === null || text === undefined) return ''; + return String(text) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); } function formatTimeAgo(date) { diff --git a/html/index.html b/html/index.html index 96d9d03..639ed8d 100644 --- a/html/index.html +++ b/html/index.html @@ -58,6 +58,7 @@ + diff --git a/html/js/core/escapeHtml.js b/html/js/core/escapeHtml.js new file mode 100644 index 0000000..592c714 --- /dev/null +++ b/html/js/core/escapeHtml.js @@ -0,0 +1,18 @@ +/** + * Shared HTML-escaping helper — the single source of truth for escaping untrusted + * (server- or user-supplied) text before it is inserted into innerHTML / jQuery .html(). + * Escapes all five HTML-significant characters, including both quote types so the + * result is safe in both element text and quoted attribute values. + */ +(function (global) { + function escapeHtml(value) { + if (value === null || value === undefined) return ''; + return String(value) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); + } + global.escapeHtml = escapeHtml; +})(typeof window !== 'undefined' ? window : this); diff --git a/html/js/network/socketHandlers.js b/html/js/network/socketHandlers.js index 2107939..6299630 100644 --- a/html/js/network/socketHandlers.js +++ b/html/js/network/socketHandlers.js @@ -288,7 +288,9 @@ const SocketHandlers = { onChatBroadcast: function(data) { const msgElement = $('
  • '); if (data.socketId) { - msgElement.html('' + data.socketId.substring(0, 6) + ': ' + data.message); + // Escape both fields — chat content is attacker-controlled. Defense in depth: + // the server also escapes, but the client must never trust that. + msgElement.html('' + escapeHtml(String(data.socketId).substring(0, 6)) + ': ' + escapeHtml(data.message)); } else { msgElement.text(data.message); } @@ -320,7 +322,7 @@ const SocketHandlers = { if (msg.timestamp) { timeStr = '[' + SocketHandlers._formatTimeAgo(new Date(msg.timestamp)) + '] '; } - msgElement.html(timeStr + '' + username + ': ' + msg.message); + msgElement.html(timeStr + '' + escapeHtml(username) + ': ' + escapeHtml(msg.message)); $('#messages').append(msgElement); }); diff --git a/html/js/ui/leaderboard.js b/html/js/ui/leaderboard.js index dc4f825..7423205 100644 --- a/html/js/ui/leaderboard.js +++ b/html/js/ui/leaderboard.js @@ -127,12 +127,15 @@ const Leaderboard = { }, _escapeHtml: function(str) { + // Delegate to the shared helper (single source of truth). Falls back to a local + // implementation if the helper somehow isn't loaded. + if (typeof window !== 'undefined' && typeof window.escapeHtml === 'function') { + return window.escapeHtml(str); + } if (!str) return ''; return String(str) - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"'); + .replace(/&/g, '&').replace(//g, '>') + .replace(/"/g, '"').replace(/'/g, '''); } }; diff --git a/src/index.js b/src/index.js index b9a541e..2d3faa1 100644 --- a/src/index.js +++ b/src/index.js @@ -54,6 +54,30 @@ let payoutRetryService = null; // Initialized later when payment system starts let batchPayoutInterval = null; // Initialized later when payment system starts let paymentExpiryInterval = null; // Initialized later when payment system starts const socketHandlers = new SocketHandlers(io, activeGames, broadcastManager, debugManager, gameModeManager, walletRPCService); +// Security headers (defense in depth alongside output escaping). +// CSP locks down where scripts/styles/connections may come from. Inline scripts/styles +// are still permitted ('unsafe-inline') because the current pages rely on them heavily; +// removing that is tracked as Phase 4.4 (nonce-based CSP). connect-src 'self' covers +// same-origin Socket.IO websockets under CSP Level 3. +app.use((req, res, next) => { + res.setHeader('Content-Security-Policy', [ + "default-src 'self'", + "script-src 'self' 'unsafe-inline' https://cdn.socket.io https://cdn.jsdelivr.net", + "style-src 'self' 'unsafe-inline'", + "img-src 'self' data:", + "font-src 'self'", + "connect-src 'self' https://smirk.cash", + "object-src 'none'", + "base-uri 'self'", + "frame-ancestors 'self'", + "form-action 'self'" + ].join('; ')); + res.setHeader('X-Content-Type-Options', 'nosniff'); + res.setHeader('X-Frame-Options', 'SAMEORIGIN'); + res.setHeader('Referrer-Policy', 'no-referrer'); + next(); +}); + // Configure static file serving const htmlPath = path.join(__dirname, '../html'); app.use(express.static(htmlPath, {