From cfe45ee2b6e18c1a9896a31518df2ea807d3e68d Mon Sep 17 00:00:00 2001 From: Matteo Susca Date: Sun, 12 Apr 2026 18:41:44 +0200 Subject: [PATCH] bugfix: server-side validation for draw events and authorization --- server/src/sockets/index.ts | 6 ++++-- server/src/store/canvas.store.ts | 5 +++++ server/test/canvas.test.ts | 16 ++++++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/server/src/sockets/index.ts b/server/src/sockets/index.ts index 60bae82..6877e80 100644 --- a/server/src/sockets/index.ts +++ b/server/src/sockets/index.ts @@ -64,19 +64,21 @@ export const setupSocket = (io: Server) => { }); socket.on(CONFIG.EVENTS.CLIENT.DRAW, ({ lobbyId, x, y, color }: DrawPayload) => { - if (lobbyId && CanvasService.draw(lobbyId, x, y, color)) { + if (!lobbyId || !socket.rooms.has(lobbyId)) return; + if (CanvasService.draw(lobbyId, x, y, color)) { broadcastToLobby(io, lobbyId, CONFIG.EVENTS.SERVER.PIXEL_UPDATE, { x, y, color }); } }); socket.on(CONFIG.EVENTS.CLIENT.DRAW_BATCH, ({ lobbyId, pixels }: DrawBatchPayload) => { - if (!lobbyId || !Array.isArray(pixels)) return; + if (!lobbyId || !socket.rooms.has(lobbyId) || !Array.isArray(pixels)) return; const updates = CanvasService.drawBatch(lobbyId, pixels); if (updates.length) broadcastToLobby(io, lobbyId, CONFIG.EVENTS.SERVER.PIXEL_UPDATE_BATCH, { pixels: updates }); }); socket.on(CONFIG.EVENTS.CLIENT.CLEAR_CANVAS, async (lobbyId: string) => { try { + if (!lobbyId || !socket.rooms.has(lobbyId)) return; const user = (socket as AuthenticatedSocket).user; if (!user || !user.id) return; diff --git a/server/src/store/canvas.store.ts b/server/src/store/canvas.store.ts index d6697fe..a1d1f21 100644 --- a/server/src/store/canvas.store.ts +++ b/server/src/store/canvas.store.ts @@ -32,6 +32,11 @@ export class CanvasStore { return false; } + // Validate color index against lobby's palette + if (color < 0 || color >= lobby.palette.length) { + return false; + } + const index = y * lobby.width + x; // Boundary check (extra safety) diff --git a/server/test/canvas.test.ts b/server/test/canvas.test.ts index c767f46..30288af 100644 --- a/server/test/canvas.test.ts +++ b/server/test/canvas.test.ts @@ -64,11 +64,23 @@ describe('Canvas Batch Processing & Conflict Resolution', () => { beforeEach(() => { vi.clearAllMocks(); canvasStore.removeLobby(LOBBY_ID); - // Init canvas with 3 colors in palette to test last-write-wins properly - canvasStore.loadLobbyToMemory(LOBBY_ID, WIDTH, HEIGHT, ['#000000', '#ffffff', '#ff0000'], new Uint8Array(WIDTH * HEIGHT)); + // Init canvas with 4 colors in palette to test last-write-wins properly + canvasStore.loadLobbyToMemory(LOBBY_ID, WIDTH, HEIGHT, ['#000000', '#ffffff', '#ff0000', '#00ff00'], new Uint8Array(WIDTH * HEIGHT)); vi.spyOn(CanvasService as any, 'scheduleSave').mockImplementation(() => { }); }); + it('should reject color indices outside the palette bounds', () => { + const x = 5; + const y = 5; + + // Palette has 4 colors (0, 1, 2, 3) + expect(CanvasService.draw(LOBBY_ID, x, y, 4)).toBe(false); // Too high + expect(CanvasService.draw(LOBBY_ID, x, y, -1)).toBe(false); // Negative + + const pixelData = canvasStore.getLobbyPixelData(LOBBY_ID); + expect(pixelData![y * WIDTH + x]).toBe(0); // Should still be 0 + }); + it('should resolve concurrent updates safely using last-write-wins', async () => { const x = 5; const y = 5;