From cf802521835cfca8e20685886aec6dcbaade9154 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 19 Sep 2026 16:09:34 -0700 Subject: [PATCH 1/2] fix(realtime): validate cursor and selection presence payloads The workflow cursor-update and selection-update handlers stored whatever object a client sent into the shared room presence hash and rebroadcast it to every peer, with no shape or size check. An authenticated user with read access to any workflow could park a multi-megabyte blob in shared Redis on every socket they opened and have the server fan it out on each presence broadcast. Both payloads are now rebuilt from a fixed field set before they reach room state or any broadcast, so unexpected keys cannot ride along - mirroring normalizeCellSelection in the table presence handler. Adds a defensive per-field length cap in updateUserActivity so future presence-bearing events inherit the bound, and marks UserPresence.cursor nullable to match the cleared-cursor value the client already sends. --- apps/realtime/src/handlers/presence.test.ts | 160 ++++++++++++++++++++ apps/realtime/src/handlers/presence.ts | 59 +++++++- apps/realtime/src/rooms/redis-manager.ts | 39 ++++- apps/realtime/src/rooms/types.ts | 3 +- 4 files changed, 253 insertions(+), 8 deletions(-) create mode 100644 apps/realtime/src/handlers/presence.test.ts diff --git a/apps/realtime/src/handlers/presence.test.ts b/apps/realtime/src/handlers/presence.test.ts new file mode 100644 index 00000000000..934e163157d --- /dev/null +++ b/apps/realtime/src/handlers/presence.test.ts @@ -0,0 +1,160 @@ +/** + * @vitest-environment node + */ +import { ROOM_TYPES } from '@sim/realtime-protocol/rooms' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { setupPresenceHandlers } from '@/handlers/presence' +import type { IRoomManager } from '@/rooms' + +const WORKFLOW_ROOM = { type: ROOM_TYPES.WORKFLOW, id: 'workflow-1' } + +const SESSION = { + userId: 'user-1', + userName: 'Test User', + avatarUrl: 'avatar.png', +} + +function createSocket() { + const handlers: Record Promise | void> = {} + const toEmit = vi.fn() + const socket = { + id: 'socket-1', + on: vi.fn((event: string, handler: (payload: unknown) => Promise | void) => { + handlers[event] = handler + }), + to: vi.fn().mockReturnValue({ emit: toEmit }), + } + return { handlers, socket, toEmit } +} + +function createRoomManager(): IRoomManager { + return { + getRoomForSocket: vi.fn().mockResolvedValue(WORKFLOW_ROOM), + getUserSession: vi.fn().mockResolvedValue(SESSION), + updateUserActivity: vi.fn().mockResolvedValue(undefined), + } as unknown as IRoomManager +} + +describe('presence handlers', () => { + let handlers: Record Promise | void> + let toEmit: ReturnType + let roomManager: IRoomManager + + beforeEach(() => { + vi.clearAllMocks() + const created = createSocket() + handlers = created.handlers + toEmit = created.toEmit + roomManager = createRoomManager() + setupPresenceHandlers(created.socket as never, roomManager) + }) + + describe('cursor-update', () => { + it('stores and broadcasts a well-formed cursor', async () => { + await handlers['cursor-update']({ cursor: { x: 12.5, y: -3 } }) + + expect(roomManager.updateUserActivity).toHaveBeenCalledWith(WORKFLOW_ROOM, 'socket-1', { + cursor: { x: 12.5, y: -3 }, + }) + expect(toEmit).toHaveBeenCalledWith( + 'cursor-update', + expect.objectContaining({ cursor: { x: 12.5, y: -3 } }) + ) + }) + + it('preserves a cleared cursor', async () => { + await handlers['cursor-update']({ cursor: null }) + + expect(roomManager.updateUserActivity).toHaveBeenCalledWith(WORKFLOW_ROOM, 'socket-1', { + cursor: null, + }) + expect(toEmit).toHaveBeenCalledWith( + 'cursor-update', + expect.objectContaining({ cursor: null }) + ) + }) + + it('strips unexpected keys instead of storing them', async () => { + await handlers['cursor-update']({ + cursor: { x: 1, y: 2, pad: 'A'.repeat(100_000) }, + }) + + expect(roomManager.updateUserActivity).toHaveBeenCalledWith(WORKFLOW_ROOM, 'socket-1', { + cursor: { x: 1, y: 2 }, + }) + const broadcast = toEmit.mock.calls[0][1] as { cursor: Record } + expect(broadcast.cursor).toEqual({ x: 1, y: 2 }) + expect(broadcast.cursor).not.toHaveProperty('pad') + }) + + it.each([ + ['an oversized string', 'A'.repeat(100_000)], + ['a non-numeric x', { x: 'A'.repeat(100_000), y: 1 }], + ['a missing y', { x: 1 }], + ['NaN coordinates', { x: Number.NaN, y: Number.NaN }], + ['Infinity coordinates', { x: Number.POSITIVE_INFINITY, y: 0 }], + ['an array', [1, 2, 3]], + ['undefined', undefined], + ])('drops %s without storing or broadcasting it', async (_label, cursor) => { + await handlers['cursor-update']({ cursor }) + + expect(roomManager.updateUserActivity).not.toHaveBeenCalled() + expect(toEmit).not.toHaveBeenCalled() + }) + }) + + describe('selection-update', () => { + it('stores and broadcasts a well-formed selection', async () => { + await handlers['selection-update']({ selection: { type: 'block', id: 'block-1' } }) + + expect(roomManager.updateUserActivity).toHaveBeenCalledWith(WORKFLOW_ROOM, 'socket-1', { + selection: { type: 'block', id: 'block-1' }, + }) + expect(toEmit).toHaveBeenCalledWith( + 'selection-update', + expect.objectContaining({ selection: { type: 'block', id: 'block-1' } }) + ) + }) + + it('keeps an id-less selection id-less', async () => { + await handlers['selection-update']({ selection: { type: 'none' } }) + + expect(roomManager.updateUserActivity).toHaveBeenCalledWith(WORKFLOW_ROOM, 'socket-1', { + selection: { type: 'none' }, + }) + }) + + it('strips unexpected keys instead of storing them', async () => { + await handlers['selection-update']({ + selection: { type: 'edge', id: 'edge-1', pad: 'A'.repeat(100_000) }, + }) + + expect(roomManager.updateUserActivity).toHaveBeenCalledWith(WORKFLOW_ROOM, 'socket-1', { + selection: { type: 'edge', id: 'edge-1' }, + }) + }) + + it.each([ + ['an unknown type', { type: 'evil', id: 'x' }], + ['a missing type', { id: 'x' }], + ['an oversized id', { type: 'block', id: 'A'.repeat(100_000) }], + ['a non-string id', { type: 'block', id: { nested: 'A'.repeat(100_000) } }], + ['null', null], + ['an oversized string', 'A'.repeat(100_000)], + ])('drops %s without storing or broadcasting it', async (_label, selection) => { + await handlers['selection-update']({ selection }) + + expect(roomManager.updateUserActivity).not.toHaveBeenCalled() + expect(toEmit).not.toHaveBeenCalled() + }) + }) + + it('does not touch room state when the socket has no room', async () => { + ;(roomManager.getRoomForSocket as ReturnType).mockResolvedValue(null) + + await handlers['cursor-update']({ cursor: { x: 1, y: 1 } }) + + expect(roomManager.updateUserActivity).not.toHaveBeenCalled() + expect(toEmit).not.toHaveBeenCalled() + }) +}) diff --git a/apps/realtime/src/handlers/presence.ts b/apps/realtime/src/handlers/presence.ts index 78b53176e2f..e57d7f17f25 100644 --- a/apps/realtime/src/handlers/presence.ts +++ b/apps/realtime/src/handlers/presence.ts @@ -1,19 +1,66 @@ import { createLogger } from '@sim/logger' +import type { CursorPosition, PresenceSelection } from '@sim/realtime-protocol/events' import { ROOM_TYPES } from '@sim/realtime-protocol/rooms' import type { AuthenticatedSocket } from '@/middleware/auth' import type { IRoomManager } from '@/rooms' const logger = createLogger('PresenceHandlers') +/** Longest accepted selection id — real ids are UUIDs/short ids; this bounds a hostile payload. */ +const MAX_SELECTION_ID_LENGTH = 200 + +/** The selection kinds a client may publish, mirroring {@link PresenceSelection}. */ +const SELECTION_TYPES = new Set(['block', 'edge', 'none']) + +/** + * Validate + whitelist an untrusted peer's cursor before it is stored and rebroadcast. + * Returns the normalized position — `null` for a legitimately cleared cursor — or + * `undefined` for anything malformed, so the caller drops it. Only `x`/`y` survive, so a + * hostile client can't amplify an oversized object through the room or the presence record. + */ +function normalizeCursor(cursor: unknown): CursorPosition | null | undefined { + if (cursor === null) return null + if (typeof cursor !== 'object') return undefined + const candidate = cursor as { x?: unknown; y?: unknown } + if (!Number.isFinite(candidate.x) || !Number.isFinite(candidate.y)) return undefined + return { x: candidate.x as number, y: candidate.y as number } +} + +/** + * Validate + whitelist an untrusted peer's selection before it is stored and rebroadcast. + * Returns the normalized selection, or `undefined` for anything malformed, so the caller + * drops it. A cleared selection is expressed as `type: 'none'`, not `null`. Rebuilding from + * a fixed field set means unexpected keys can't ride along into the shared presence record. + */ +function normalizeSelection(selection: unknown): PresenceSelection | undefined { + if (typeof selection !== 'object' || selection === null) return undefined + const candidate = selection as { type?: unknown; id?: unknown } + if (!SELECTION_TYPES.has(candidate.type as PresenceSelection['type'])) return undefined + if ( + candidate.id !== undefined && + (typeof candidate.id !== 'string' || candidate.id.length > MAX_SELECTION_ID_LENGTH) + ) { + return undefined + } + return { + type: candidate.type as PresenceSelection['type'], + ...(typeof candidate.id === 'string' ? { id: candidate.id } : {}), + } +} + export function setupPresenceHandlers(socket: AuthenticatedSocket, roomManager: IRoomManager) { - socket.on('cursor-update', async ({ cursor }) => { + socket.on('cursor-update', async ({ cursor: rawCursor }: { cursor: unknown }) => { try { + // Drop a malformed/oversized cursor from an untrusted peer before it is stored or + // rebroadcast (`undefined` = invalid; `null` = a legitimately cleared cursor). + const cursor = normalizeCursor(rawCursor) + if (cursor === undefined) return + const room = await roomManager.getRoomForSocket(socket.id, ROOM_TYPES.WORKFLOW) const session = await roomManager.getUserSession(socket.id) if (!room || !session) return - // Update cursor in room state await roomManager.updateUserActivity(room, socket.id, { cursor }) // Broadcast to other users in the room (workflow room name is the bare id) @@ -29,14 +76,18 @@ export function setupPresenceHandlers(socket: AuthenticatedSocket, roomManager: } }) - socket.on('selection-update', async ({ selection }) => { + socket.on('selection-update', async ({ selection: rawSelection }: { selection: unknown }) => { try { + // Drop a malformed/oversized selection from an untrusted peer before it is stored + // or rebroadcast. + const selection = normalizeSelection(rawSelection) + if (selection === undefined) return + const room = await roomManager.getRoomForSocket(socket.id, ROOM_TYPES.WORKFLOW) const session = await roomManager.getUserSession(socket.id) if (!room || !session) return - // Update selection in room state await roomManager.updateUserActivity(room, socket.id, { selection }) // Broadcast to other users in the room (workflow room name is the bare id) diff --git a/apps/realtime/src/rooms/redis-manager.ts b/apps/realtime/src/rooms/redis-manager.ts index 7282f4a6778..b3ff429df14 100644 --- a/apps/realtime/src/rooms/redis-manager.ts +++ b/apps/realtime/src/rooms/redis-manager.ts @@ -122,6 +122,39 @@ redis.call('EXPIRE', socketSessionKey, sessionTtl) return 1 ` +/** + * Ceiling on the JSON length of a single presence field. Every legitimate payload is far + * below this — a cursor is tens of characters, and the largest handler-bounded selection + * (two cell refs whose ids cap at 200) stays under 500 — so this never trims real presence. + * It is a backstop for presence-bearing events whose handler validation is missing or + * regresses, capping what one socket can park in the shared room hash and fan out to peers. + */ +const MAX_PRESENCE_FIELD_LENGTH = 4096 + +/** + * Serialize one presence field for the activity script. Returns `''` when there is no + * update (the script skips the field) and, defensively, when the value exceeds + * {@link MAX_PRESENCE_FIELD_LENGTH} — dropping just that field rather than the whole + * update, so a single oversized field can't suppress the others or the activity refresh. + */ +function serializePresenceField( + field: 'cursor' | 'selection' | 'cell', + value: unknown, + socketId: string +): string { + if (value === undefined) return '' + const serialized = JSON.stringify(value) + if (serialized.length > MAX_PRESENCE_FIELD_LENGTH) { + logger.warn('Dropping oversized presence field', { + field, + socketId, + length: serialized.length, + }) + return '' + } + return serialized +} + /** * Redis-backed room manager for multi-pod deployments. Domain-neutral: keyed by * {@link RoomRef}, supports a socket in multiple rooms (one per {@link RoomType}). @@ -370,14 +403,14 @@ export class RedisRoomManager implements IRoomManager { keys: [KEYS.roomUsers(room), KEYS.socketRooms(socketId), KEYS.socketSession(socketId)], arguments: [ socketId, - updates.cursor !== undefined ? JSON.stringify(updates.cursor) : '', - updates.selection !== undefined ? JSON.stringify(updates.selection) : '', + serializePresenceField('cursor', updates.cursor, socketId), + serializePresenceField('selection', updates.selection, socketId), (updates.lastActivity ?? Date.now()).toString(), SOCKET_ROOMS_TTL.toString(), SESSION_TTL.toString(), // Trailing arg (ARGV[7]) so existing indices stay stable. `null` (cleared // selection) serializes to 'null'; `undefined` (no cell change) to '' (skip). - updates.cell !== undefined ? JSON.stringify(updates.cell) : '', + serializePresenceField('cell', updates.cell, socketId), ], }) } catch (error) { diff --git a/apps/realtime/src/rooms/types.ts b/apps/realtime/src/rooms/types.ts index f1a67d66abb..9ef4e77fa70 100644 --- a/apps/realtime/src/rooms/types.ts +++ b/apps/realtime/src/rooms/types.ts @@ -17,7 +17,8 @@ export interface UserPresence { joinedAt: number lastActivity: number role: string - cursor?: { x: number; y: number } + /** The viewer's pointer position. `null` clears it (the pointer left the canvas). */ + cursor?: { x: number; y: number } | null selection?: { type: 'block' | 'edge' | 'none'; id?: string } /** The viewer's current table cell selection, for table presence rooms. */ cell?: TableCellSelection From e2c57dbaaa97aae25fb440bc392012009a5ae1b7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 19 Sep 2026 16:16:28 -0700 Subject: [PATCH 2/2] fix(realtime): measure presence field cap in utf-8 bytes The cap compared UTF-16 code units against a byte budget, so a multi-byte payload could pass the check and still land several times larger in the room hash. It now measures the UTF-8 bytes Redis actually stores. Raises the ceiling to 16384. A table cell selection carries four ids capped at 200 characters each, and multi-byte characters plus JSON escaping can expand a legitimate worst case to roughly 5 KB - above the previous 4096, so the old bound could have dropped real presence. --- apps/realtime/src/rooms/redis-manager.ts | 28 +++++++++++++----------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/apps/realtime/src/rooms/redis-manager.ts b/apps/realtime/src/rooms/redis-manager.ts index b3ff429df14..17d67377531 100644 --- a/apps/realtime/src/rooms/redis-manager.ts +++ b/apps/realtime/src/rooms/redis-manager.ts @@ -123,18 +123,23 @@ return 1 ` /** - * Ceiling on the JSON length of a single presence field. Every legitimate payload is far - * below this — a cursor is tens of characters, and the largest handler-bounded selection - * (two cell refs whose ids cap at 200) stays under 500 — so this never trims real presence. - * It is a backstop for presence-bearing events whose handler validation is missing or - * regresses, capping what one socket can park in the shared room hash and fan out to peers. + * Ceiling on a single presence field, measured in the UTF-8 bytes Redis actually stores + * rather than UTF-16 code units, so a multi-byte payload can't pass a character-based check + * and still land several times larger in the room hash. + * + * The largest legitimate payload is a table cell selection: four ids capped at 200 + * characters each. Multi-byte characters and JSON escaping can expand those well past + * their character count, so the realistic worst case approaches 5 KB — this sits comfortably + * above that, and a backstop that could trim real presence would be worse than a loose one. + * It bounds what one socket can park in the shared room hash and fan out to every peer when + * a presence-bearing event's handler validation is missing or regresses. */ -const MAX_PRESENCE_FIELD_LENGTH = 4096 +const MAX_PRESENCE_FIELD_BYTES = 16384 /** * Serialize one presence field for the activity script. Returns `''` when there is no * update (the script skips the field) and, defensively, when the value exceeds - * {@link MAX_PRESENCE_FIELD_LENGTH} — dropping just that field rather than the whole + * {@link MAX_PRESENCE_FIELD_BYTES} — dropping just that field rather than the whole * update, so a single oversized field can't suppress the others or the activity refresh. */ function serializePresenceField( @@ -144,12 +149,9 @@ function serializePresenceField( ): string { if (value === undefined) return '' const serialized = JSON.stringify(value) - if (serialized.length > MAX_PRESENCE_FIELD_LENGTH) { - logger.warn('Dropping oversized presence field', { - field, - socketId, - length: serialized.length, - }) + const bytes = Buffer.byteLength(serialized, 'utf8') + if (bytes > MAX_PRESENCE_FIELD_BYTES) { + logger.warn('Dropping oversized presence field', { field, socketId, bytes }) return '' } return serialized