diff --git a/.gitignore b/.gitignore index 4b503c37..6b2a32ec 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,7 @@ # misc .DS_Store *.pem +tmpclaude* logs #visual studio diff --git a/api/eloRank.js b/api/eloRank.js index 98045c55..8ed3a34d 100644 --- a/api/eloRank.js +++ b/api/eloRank.js @@ -33,9 +33,11 @@ export default async function handler(req, res) { try { let user; - if(username) { + if(username && typeof username === 'string') { + // Prevent NoSQL injection - username must be a string user = await User.findOne({ username: username }).collation(USERNAME_COLLATION).cache(120); - } else if(secret) { + } else if(secret && typeof secret === 'string') { + // Prevent NoSQL injection - secret must be a string user = await User.findOne({ secret }).cache(120); } diff --git a/api/googleAuth.js b/api/googleAuth.js index 3a8c14db..032b0069 100644 --- a/api/googleAuth.js +++ b/api/googleAuth.js @@ -107,7 +107,8 @@ export default async function handler(req, res) { const { code, secret } = req.body; if (!code) { - if(!secret) { + // Prevent NoSQL injection - secret must be a string + if(!secret || typeof secret !== 'string') { return res.status(400).json({ error: 'Invalid' }); } diff --git a/api/leaderboard.js b/api/leaderboard.js index 720e7c60..fe065346 100644 --- a/api/leaderboard.js +++ b/api/leaderboard.js @@ -113,6 +113,11 @@ export default async function handler(req, res) { const isXp = req.query.mode === 'xp'; console.log(`[API] leaderboard: mode=${isXp ? 'xp' : 'elo'}, pastDay=${pastDay}, user=${myUsername || 'none'}`); + // Prevent NoSQL injection - username must be a string if provided + if (myUsername && typeof myUsername !== 'string') { + return res.status(400).json({ message: 'Invalid username' }); + } + if (req.method !== 'GET') { return res.status(405).json({ message: 'Method not allowed' }); } diff --git a/api/map/mapHome.js b/api/map/mapHome.js index 6ffe846d..5ed10fe9 100644 --- a/api/map/mapHome.js +++ b/api/map/mapHome.js @@ -40,12 +40,13 @@ export default async function handler(req, res) { // Skip user lookup for anonymous requests if(secret && !isAnon) { - const startUser = Date.now(); - user = await User.findOne({ secret: secret }); - timings.userLookup = Date.now() - startUser; + // Prevent NoSQL injection - validate secret type BEFORE the query if(typeof secret !== 'string') { return res.status(400).json({ message: 'Invalid input' }); } + const startUser = Date.now(); + user = await User.findOne({ secret: secret }); + timings.userLookup = Date.now() - startUser; if(!user) { return res.status(404).json({ message: 'User not found' }); } diff --git a/components/home.js b/components/home.js index 53b750a9..42d90f29 100644 --- a/components/home.js +++ b/components/home.js @@ -118,6 +118,7 @@ export default function Home({ }) { const [dismissedNameChangeBanner, setDismissedNameChangeBanner] = useState(false) const [dismissedBanBanner, setDismissedBanBanner] = useState(false) const [timeOffset, setTimeOffset] = useState(0) + const timeSyncRef = useRef({ bestRtt: Infinity, lastSyncAt: 0, lastServerNow: 0 }) const [loginQueued, setLoginQueued] = useState(false); const [options, setOptions] = useState({ }); @@ -967,6 +968,37 @@ export default function Home({ }) { const [multiplayerChatOpen, setMultiplayerChatOpen] = useState(false); const [multiplayerChatEnabled, setMultiplayerChatEnabled] = useState(false); + const updateTimeOffsetFromSync = (serverNow, clientSentAt) => { + if (!serverNow || !clientSentAt) return; + const now = Date.now(); + const rtt = Math.max(0, now - clientSentAt); + const offset = serverNow - (clientSentAt + rtt / 2); + const sync = timeSyncRef.current; + const tooOld = now - sync.lastSyncAt > 60000; + const betterRtt = rtt <= sync.bestRtt + 25; + if (sync.lastSyncAt === 0 || betterRtt || tooOld) { + const prevBestRtt = sync.bestRtt; + sync.bestRtt = Math.min(sync.bestRtt, rtt); + sync.lastSyncAt = now; + sync.lastServerNow = serverNow; + if (window.debugTimeSync) { + console.log("[TimeSync] update", { + offset, + rtt, + serverNow, + clientSentAt, + prevBestRtt + }); + } + setTimeOffset(offset); + } + }; + + const sendTimeSync = () => { + if (!ws || ws.readyState !== WebSocket.OPEN) return; + ws.send(JSON.stringify({ type: "timeSync", clientSentAt: Date.now() })); + }; + // Auto-close connection error modal when connected useEffect(() => { @@ -985,6 +1017,27 @@ export default function Home({ }) { } }, [session?.token?.secret, ws]) + useEffect(() => { + if (!ws) return; + timeSyncRef.current = { bestRtt: Infinity, lastSyncAt: 0, lastServerNow: 0 }; + setTimeOffset(0); + sendTimeSync(); + const interval = setInterval(() => { + sendTimeSync(); + }, 30000); + return () => clearInterval(interval); + }, [ws]) + + useEffect(() => { + const handleVisibility = () => { + if (document.visibilityState === "visible") { + sendTimeSync(); + } + }; + document.addEventListener("visibilitychange", handleVisibility); + return () => document.removeEventListener("visibilitychange", handleVisibility); + }, [ws]) + const { t: text } = useTranslation("common"); useEffect(() => { @@ -1371,10 +1424,23 @@ export default function Home({ }) { } if (data.type === "t") { const offset = data.t - Date.now(); - if (Math.abs(offset) > 1000 && ((Math.abs(offset) < Math.abs(timeOffset)) || !timeOffset)) { + const sync = timeSyncRef.current; + const now = Date.now(); + const useFallback = sync.lastSyncAt === 0 || (now - sync.lastSyncAt) > 60000; + if (useFallback && Math.abs(offset) < 300000) { + if (window.debugTimeSync) { + console.log("[TimeSync] fallback", { + offset, + serverNow: data.t, + lastSyncAt: sync.lastSyncAt + }); + } setTimeOffset(offset) } } + if (data.type === "timeSync") { + updateTimeOffsetFromSync(data.serverNow, data.clientSentAt); + } if (data.type === "elo") { setEloData((prev) => ({ diff --git a/serverUtils/validateSecret.js b/serverUtils/validateSecret.js index fb90265d..feffb5bf 100644 --- a/serverUtils/validateSecret.js +++ b/serverUtils/validateSecret.js @@ -1,4 +1,9 @@ export default async function validateSecret(secret, User) { + // Prevent NoSQL injection - secret must be a string + if (typeof secret !== 'string') { + return null; + } + const user = await User.findOne({ secret }); diff --git a/ws/classes/Game.js b/ws/classes/Game.js index f4411bf2..a03e07c5 100644 --- a/ws/classes/Game.js +++ b/ws/classes/Game.js @@ -90,6 +90,7 @@ export default class Game { eloChanges: this.eloChanges, pIds: this.pIds, accountIds: this.accountIds, + oldElos: this.oldElos, gameCount: this.gameCount, location: this.location, saveInProgress: this.saveInProgress, @@ -414,7 +415,6 @@ export default class Game { // For ranked duels: if someone leaves during "getready" (countdown before first round), // cancel the game without ELO penalties - no actual gameplay has happened yet const isPreGameLeave = this.public && this.duel && this.state === 'getready'; - // Track disconnection for ranked duels (only if actual gameplay has started) if(this.public && this.duel && !isPreGameLeave) { this.disconnectedPlayer = tag; @@ -462,7 +462,7 @@ export default class Game { console.log('Cannot start game: not in waiting state', this.state); return; } - + if (Object.keys(this.players).length < 2) { console.log('Cannot start game: not enough players', Object.keys(this.players).length); if (hostPlayer) { @@ -474,7 +474,7 @@ export default class Game { } return; } - + if (this.rounds !== this.locations.length) { console.log('Cannot start game: locations not loaded', this.rounds, this.locations.length); if (hostPlayer) { @@ -555,7 +555,7 @@ export default class Game { } - if (allFinal && (this.nextEvtTime - Date.now()) > 5000) { + if (allFinal && (this.nextEvtTime - Date.now()) > 1000) { this.nextEvtTime = Date.now() + 1000; this.sendStateUpdate(); } @@ -694,7 +694,7 @@ export default class Game { p.send(json); } } - end(leftUser) { + async end(leftUser) { console.log(`Ending game ${this.id} - duel: ${this.duel}, public: ${this.public}, players: ${Object.keys(this.players).length}`); // For duels, only save the final round if it was actually completed (all players made guesses) // For regular games, save if the round was started but not yet saved @@ -773,21 +773,26 @@ export default class Game { } - let p1NewElo = null; - let p2NewElo = null; - let p1OldElo = p1obj?.elo || null; - let p2OldElo = p2obj?.elo || null; + const p1EloResult = await User.findById(this.accountIds.p1).select('elo').lean(); + const p2EloResult = await User.findById(this.accountIds.p2).select('elo').lean(); + // Use DB value if available, otherwise fall back to stored oldElos from game creation + // This prevents null ELO bugs while still handling external ELO updates + let p1OldElo = p1EloResult?.elo ?? this.oldElos?.p1 ?? null; + let p2OldElo = p2EloResult?.elo ?? this.oldElos?.p2 ?? null; + + let p1NewElo = p1OldElo; + let p2NewElo = p2OldElo; // elo changes - if(this.eloChanges) { + if(this.eloChanges && p1OldElo && p2OldElo) { if(draw) { const changes = this.eloChanges.draw; // { newRating1, newRating2 } - p1NewElo = changes.newRating1; - p2NewElo = changes.newRating2; + p1NewElo += changes.newRating1; + p2NewElo += changes.newRating2; if(p1obj) { @@ -797,27 +802,27 @@ export default class Game { } if(p2obj) { - p2obj.setElo(changes.newRating2, { draw: true, oldElo: p2OldElo }); + p2obj.setElo(p2NewElo, { draw: true, oldElo: p2OldElo }); } else { - setElo(this.accountIds.p2, changes.newRating2, { draw: true, oldElo: p2OldElo }); + setElo(this.accountIds.p2, p2NewElo, { draw: true, oldElo: p2OldElo }); } } else if(winner) { const changes = this.eloChanges[winner.id]; // { newRating1, newRating2 } - p1NewElo = changes.newRating1; - p2NewElo = changes.newRating2; + p1NewElo += changes.newRating1; + p2NewElo += changes.newRating2; if(p1obj) { - p1obj.setElo(changes.newRating1, { winner: winner.tag === 'p1', oldElo: p1OldElo }); + p1obj.setElo(p1NewElo, { winner: winner.tag === 'p1', oldElo: p1OldElo }); } else { - setElo(this.accountIds.p1, changes.newRating1, { winner: winner.tag === 'p1', oldElo: p1OldElo }); + setElo(this.accountIds.p1, p1NewElo, { winner: winner.tag === 'p1', oldElo: p1OldElo }); } if(p2obj) { - p2obj.setElo(changes.newRating2, { winner: winner.tag === 'p2', oldElo: p2OldElo }); + p2obj.setElo(p2NewElo, { winner: winner.tag === 'p2', oldElo: p2OldElo }); } else { - setElo(this.accountIds.p2, changes.newRating2, { winner: winner.tag === 'p2', oldElo: p2OldElo }); + setElo(this.accountIds.p2, p2NewElo, { winner: winner.tag === 'p2', oldElo: p2OldElo }); } } @@ -852,7 +857,7 @@ export default class Game { } // Save duel game to MongoDB for history tracking - if(this.duel && this.accountIds?.p1 && this.accountIds?.p2) { + if(this.duel && this.accountIds?.p1 && this.accountIds?.p2 && p1OldElo && p2OldElo) { this.saveInProgress = true; const p1Xp = this.calculatePlayerXp(this.pIds?.p1); const p2Xp = this.calculatePlayerXp(this.pIds?.p2); @@ -1128,7 +1133,7 @@ export default class Game { // Create player summaries sorted by total points (highest first) - include ALL players // Calculate XP for public games only (not parties), and only for registered users (not guests) const awardXp = this.public; - + const playerSummaries = allPlayers .map(player => { const playerXp = (awardXp && player.accountId) ? this.calculatePlayerXp(player.id) : 0; diff --git a/ws/classes/Player.js b/ws/classes/Player.js index 17da5c80..fe0f4ca8 100644 --- a/ws/classes/Player.js +++ b/ws/classes/Player.js @@ -161,7 +161,8 @@ export default class Player { // account verification if((!json.secret) ||(json.secret === 'not_logged_in')) { if(!this.verified) { - if(json.rejoinCode) { + if(typeof json.rejoinCode === 'string' && json.rejoinCode.includes('-')) { + // Only accept UUID format rejoinCodes (contain dashes), reject MongoDB ObjectIds const dcPlayerId = disconnectedPlayers.get(json.rejoinCode); if(dcPlayerId) { handleReconnect(dcPlayerId, json.rejoinCode); diff --git a/ws/ws.js b/ws/ws.js index 7f312fb9..f85ca477 100644 --- a/ws/ws.js +++ b/ws/ws.js @@ -574,6 +574,16 @@ app.ws('/wg', { player.lastPong = Date.now(); return; } + if (json.type === "timeSync") { + if (typeof json.clientSentAt === "number") { + player.send({ + type: "timeSync", + clientSentAt: json.clientSentAt, + serverNow: Date.now() + }); + } + return; + } if (json.type === 'verify') { player.verify(json); return; @@ -1566,10 +1576,23 @@ try { const eloDraw = calculateOutcomes(p1.elo, p2.elo, 0.5); const eloP2Win = calculateOutcomes(p1.elo, p2.elo, 0); + const deltaP1Win = {newRating1: eloP1Win.newRating1 - p1.elo, newRating2: eloP1Win.newRating2 - p2.elo}; + const deltaP2Win = {newRating1: eloP2Win.newRating1 - p1.elo, newRating2: eloP2Win.newRating2 - p2.elo}; + const deltaDraw = {newRating1: eloDraw.newRating1 - p1.elo, newRating2: eloDraw.newRating2 - p2.elo}; + game.eloChanges = { - [p1.id]: eloP1Win, - [p2.id]: eloP2Win, - draw: eloDraw + [p1.id]: deltaP1Win, + [p2.id]: deltaP2Win, + draw: deltaDraw + } + + game.oldElos = { + p1: p1.elo, + p2: p2.elo + } + + if (process.env.DEBUG_ELO_CHANGES === 'true') { + console.log('game.eloChanges', game.eloChanges, game.oldElos); } if(p1.elo > 2000 && p2.elo > 2000) {