Skip to content

Commit ae5bc7b

Browse files
authored
Merge pull request #126 from pixie-git/bugfix/server-side-draw-validation
bugfix: server-side validation for draw events and authorization
2 parents 7cf16eb + cfe45ee commit ae5bc7b

3 files changed

Lines changed: 23 additions & 4 deletions

File tree

server/src/sockets/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,21 @@ export const setupSocket = (io: Server) => {
6464
});
6565

6666
socket.on(CONFIG.EVENTS.CLIENT.DRAW, ({ lobbyId, x, y, color }: DrawPayload) => {
67-
if (lobbyId && CanvasService.draw(lobbyId, x, y, color)) {
67+
if (!lobbyId || !socket.rooms.has(lobbyId)) return;
68+
if (CanvasService.draw(lobbyId, x, y, color)) {
6869
broadcastToLobby(io, lobbyId, CONFIG.EVENTS.SERVER.PIXEL_UPDATE, { x, y, color });
6970
}
7071
});
7172

7273
socket.on(CONFIG.EVENTS.CLIENT.DRAW_BATCH, ({ lobbyId, pixels }: DrawBatchPayload) => {
73-
if (!lobbyId || !Array.isArray(pixels)) return;
74+
if (!lobbyId || !socket.rooms.has(lobbyId) || !Array.isArray(pixels)) return;
7475
const updates = CanvasService.drawBatch(lobbyId, pixels);
7576
if (updates.length) broadcastToLobby(io, lobbyId, CONFIG.EVENTS.SERVER.PIXEL_UPDATE_BATCH, { pixels: updates });
7677
});
7778

7879
socket.on(CONFIG.EVENTS.CLIENT.CLEAR_CANVAS, async (lobbyId: string) => {
7980
try {
81+
if (!lobbyId || !socket.rooms.has(lobbyId)) return;
8082
const user = (socket as AuthenticatedSocket).user;
8183
if (!user || !user.id) return;
8284

server/src/store/canvas.store.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ export class CanvasStore {
3232
return false;
3333
}
3434

35+
// Validate color index against lobby's palette
36+
if (color < 0 || color >= lobby.palette.length) {
37+
return false;
38+
}
39+
3540
const index = y * lobby.width + x;
3641

3742
// Boundary check (extra safety)

server/test/canvas.test.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,23 @@ describe('Canvas Batch Processing & Conflict Resolution', () => {
6464
beforeEach(() => {
6565
vi.clearAllMocks();
6666
canvasStore.removeLobby(LOBBY_ID);
67-
// Init canvas with 3 colors in palette to test last-write-wins properly
68-
canvasStore.loadLobbyToMemory(LOBBY_ID, WIDTH, HEIGHT, ['#000000', '#ffffff', '#ff0000'], new Uint8Array(WIDTH * HEIGHT));
67+
// Init canvas with 4 colors in palette to test last-write-wins properly
68+
canvasStore.loadLobbyToMemory(LOBBY_ID, WIDTH, HEIGHT, ['#000000', '#ffffff', '#ff0000', '#00ff00'], new Uint8Array(WIDTH * HEIGHT));
6969
vi.spyOn(CanvasService as any, 'scheduleSave').mockImplementation(() => { });
7070
});
7171

72+
it('should reject color indices outside the palette bounds', () => {
73+
const x = 5;
74+
const y = 5;
75+
76+
// Palette has 4 colors (0, 1, 2, 3)
77+
expect(CanvasService.draw(LOBBY_ID, x, y, 4)).toBe(false); // Too high
78+
expect(CanvasService.draw(LOBBY_ID, x, y, -1)).toBe(false); // Negative
79+
80+
const pixelData = canvasStore.getLobbyPixelData(LOBBY_ID);
81+
expect(pixelData![y * WIDTH + x]).toBe(0); // Should still be 0
82+
});
83+
7284
it('should resolve concurrent updates safely using last-write-wins', async () => {
7385
const x = 5;
7486
const y = 5;

0 commit comments

Comments
 (0)