fix(rpc): prevent client id spoofing

rpc/src/core/base.ts

@@ -4,13 +4,14 @@ import {
 	notRegisteredMessage,
 	RPCError,
 } from '../utils/errors'
-import { generateUUID } from '../utils/funcs'
+import { generateUUID, isRPCState, parse } from '../utils/funcs'
 import { Pending } from '../utils/pending'
 import {
 	type RPCConfig,
 	type RPCEnvironment,
 	RPCErrors,
 	type RPCState,
+	type RPCStateRaw,
 } from '../utils/types'
 
 export class RPCInstanceBase {
@@ -76,15 +77,46 @@ export class RPCInstanceBase {
 		return this._emitterLocal.emit<R>(event, ...args)
 	}
 
-	/** Settles the call waiting for `response`; ignores late or unexpected ones */
+	/**
-	protected settle(response: RPCState): void {
+	 * Settles the call waiting for `response`; ignores late or unexpected ones.
+	 *
+	 * @param peer - who sent the response (server: `source`)
+	 */
+	protected settle(response: RPCState, peer?: number): void {
 		const found = response.error
-			? this._pending.reject(response.uuid, RPCError.fromResponse(response))
+			? this._pending.reject(
-			: this._pending.resolve(response.uuid, response.data?.[0])
+					response.uuid,
+					RPCError.fromResponse(response),
+					peer,
+				)
+			: this._pending.resolve(response.uuid, response.data?.[0], peer)
 
 		if (!found) this.logIgnored(response)
 	}
 
+	/**
+	 * Parses and validates an incoming payload. Anything else sent on the RPC
+	 * channels (broken JSON, other shapes) is dropped: `null`.
+	 */
+	protected accept(input: RPCStateRaw | unknown): RPCState | null {
+		const payload =
+			typeof input === 'string'
+				? parse(input as RPCStateRaw)
+				: isRPCState(input)
+					? input
+					: null
+
+		if (!payload) {
+			this.log(`dropped invalid payload ${String(input).slice(0, 200)}`)
+			return null
+		}
+
+		this.log(
+			`accepted ${payload.type} ${payload.event} from ${payload.calledFrom}`,
+		)
+		return payload
+	}
+
 	/**
 	 * Runs the listener for `request` and builds the response to send back.
 	 * Never throws: a missing listener or a thrown error ends up in `error`.

rpc/src/core/client.ts

@@ -86,7 +86,6 @@ export class RPCInstanceClient extends RPCInstanceBase {
 				return this.dispatch(this._emitterWeb, payload)
 			}
 			if (payload.calledTo === 'server') {
-				payload.player = GetPlayerServerId(PlayerId())
 				emitNet(RPCEvents.LISTENER_WEB, stringify(payload))
 
 				return this._pending.wait<RPCState>(payload)
@@ -100,7 +99,6 @@ export class RPCInstanceClient extends RPCInstanceBase {
 				return { status: 'ok' }
 			}
 			if (payload.calledTo === 'server') {
-				payload.player = GetPlayerServerId(PlayerId())
 				emitNet(RPCEvents.LISTENER_WEB, stringify(payload))
 
 				return { status: 'ok' }
@@ -135,12 +133,7 @@ export class RPCInstanceClient extends RPCInstanceBase {
 		Arguments extends Parameters<s.RPCEvents_ClientServer[EventName]>,
 		Response extends ReturnType<s.RPCEvents_ClientServer[EventName]>,
 	>(eventName: EventName, ...args: Arguments): Promise<Awaited<Response>> {
-		const payload = this.request(
+		const payload = this.request(eventName, 'server', args, null)
-			eventName,
-			'server',
-			args,
-			GetPlayerServerId(PlayerId()),
-		)
 
 		emitNet(RPCEvents.LISTENER_CLIENT, stringify(payload))
 
@@ -173,7 +166,7 @@ export class RPCInstanceClient extends RPCInstanceBase {
 		Arguments extends Parameters<s.RPCEvents_ClientWebview[EventName]>,
 		Response extends ReturnType<s.RPCEvents_ClientWebview[EventName]>,
 	>(eventName: EventName, ...args: Arguments): Promise<Awaited<Response>> {
-		const payload = this.request(eventName, 'webview', args, PlayerId())
+		const payload = this.request(eventName, 'webview', args, null)
 
 		this._sendWebMessage({
 			origin: RPCEvents.LISTENER_CLIENT,

rpc/src/core/server.ts

@@ -25,71 +25,44 @@ export class RPCInstanceServer extends RPCInstanceBase {
 
 		console.log('[RPC] Initialized Server')
 
-		onNet(RPCEvents.LISTENER_CLIENT, this._handleClient.bind(this))
+		// `source` must be read synchronously, before any await
-		onNet(RPCEvents.LISTENER_WEB, this._handleWeb.bind(this))
+		onNet(RPCEvents.LISTENER_CLIENT, (raw: RPCStateRaw) =>
+			this._handle(raw, source, 'client', this._emitterClient),
+		)
+		onNet(RPCEvents.LISTENER_WEB, (raw: RPCStateRaw) =>
+			this._handle(raw, source, 'webview', this._emitterWeb),
+		)
 	}
 
 	// ===== HANDLERS =====
 
-	private async _handleClient(payloadRaw: RPCStateRaw) {
+	/**
+	 * Handles a payload from a client or its webview.
+	 *
+	 * @param player - FiveM `source` of the net event: the only trusted player id,
+	 *   whatever the payload claims
+	 */
+	private async _handle(
+		payloadRaw: RPCStateRaw,
+		player: number,
+		from: 'client' | 'webview',
+		emitter: Emitter,
+	) {
 		const payload = this.accept(payloadRaw)
-		if (!payload) return
+		if (!payload || payload.calledFrom !== from) return
 
-		if (payload.calledFrom === 'client') {
+		// not sent by a player, e.g. a server-side trigger of the RPC channel
-			if (payload.type === 'event') {
+		if (!(player > 0)) {
-				if (payload.player === null || payload.player === -1) {
+			this.log(`dropped ${payload.event}: no player source`)
-					// nobody to reply to, the caller times out
-					console.error(
-						new RPCError(
-							RPCErrors.NO_PLAYER,
-							`${RPCErrors.NO_PLAYER}: "${payload.event}" from ${payload.calledFrom}`,
-						),
-					)
 			return
 		}
+		payload.player = player
 
-				const response = await this.dispatch(
-					this._emitterClient,
-					payload,
-					payload.player,
-				)
-
-				emitNet(RPCEvents.LISTENER_SERVER, response.player, stringify(response))
-			}
-			if (payload.type === 'response') {
-				this.settle(payload)
-			}
-		}
-	}
-
-	private async _handleWeb(payloadRaw: RPCStateRaw) {
-		const payload = this.accept(payloadRaw)
-		if (!payload) return
-
-		if (payload.calledFrom === 'webview') {
 		if (payload.type === 'event') {
-				if (payload.player === null || payload.player === -1) {
+			const response = await this.dispatch(emitter, payload, player)
-					// nobody to reply to, the caller times out
+			emitNet(RPCEvents.LISTENER_SERVER, player, stringify(response))
-					console.error(
+		} else {
-						new RPCError(
+			this.settle(payload, player)
-							RPCErrors.NO_PLAYER,
-							`${RPCErrors.NO_PLAYER}: "${payload.event}" from ${payload.calledFrom}`,
-						),
-					)
-					return
-				}
-
-				const response = await this.dispatch(
-					this._emitterWeb,
-					payload,
-					payload.player,
-				)
-
-				emitNet(RPCEvents.LISTENER_SERVER, response.player, stringify(response))
-			}
-			if (payload.type === 'response') {
-				this.settle(payload)
-			}
 		}
 	}
 
@@ -128,7 +101,7 @@ export class RPCInstanceServer extends RPCInstanceBase {
 
 		emitNet(RPCEvents.LISTENER_SERVER, player, stringify(payload))
 
-		return this._pending.wait<Awaited<Response>>(payload)
+		return this._pending.wait<Awaited<Response>>(payload, player)
 	}
 
 	public async emitClientEveryone<
@@ -175,7 +148,7 @@ export class RPCInstanceServer extends RPCInstanceBase {
 
 		emitNet(RPCEvents.LISTENER_SERVER, player, stringify(payload))
 
-		return this._pending.wait<Awaited<Response>>(payload)
+		return this._pending.wait<Awaited<Response>>(payload, player)
 	}
 
 	// ===== SELF =====

rpc/src/fivem.d.ts

@@ -21,11 +21,14 @@ declare function RegisterCommand<A extends unknown[]>(
 	restricted: boolean,
 ): void
 
+// ===== SERVER =====
+
+/** Player that sent the current net event. Only valid synchronously in the handler */
+declare var source: number
+
 // ===== CLIENT =====
 
 declare function RegisterNuiCallbackType(callbackType: string): void
-declare function GetPlayerServerId(player: number): number
-declare function PlayerId(): number
 declare function SetNuiFocus(hasFocus: boolean, hasCursor: boolean): void
 declare function SendNuiMessage(jsonString: string): boolean
 

rpc/src/utils/pending.ts

@@ -5,6 +5,8 @@ type Call = {
 	resolve: (data: unknown) => void
 	reject: (error: Error) => void
 	timer: ReturnType<typeof setTimeout> | undefined
+	/** Only a response from this peer settles the call (server: target player) */
+	peer: number | undefined
 }
 
 /** Calls waiting for a response, keyed by payload uuid. */
@@ -14,13 +16,18 @@ export class Pending {
 	/** @param timeout - ms before a call rejects, `0` or less disables it */
 	constructor(private readonly _timeout: number) {}
 
-	/** Waits for the response to `request`, rejects with `RPCErrors.TIMEOUT` */
+	/**
-	public wait<R>(request: RPCState): Promise<R> {
+	 * Waits for the response to `request`, rejects with `RPCErrors.TIMEOUT`.
+	 *
+	 * @param peer - if set, only a response from this peer settles the call
+	 */
+	public wait<R>(request: RPCState, peer?: number): Promise<R> {
 		return new Promise<R>((resolve, reject) => {
 			const timer =
 				this._timeout > 0
 					? setTimeout(
-							() => this.reject(request.uuid, this._timeoutError(request)),
+							() =>
+								this.reject(request.uuid, this._timeoutError(request), peer),
 							this._timeout,
 						)
 					: undefined
@@ -29,30 +36,31 @@ export class Pending {
 				resolve: resolve as (data: unknown) => void,
 				reject,
 				timer,
+				peer,
 			})
 		})
 	}
 
-	/** @returns `false` if no call waits for this uuid (late or unexpected response) */
+	/** @returns `false` if no call from `peer` waits for this uuid (late or unexpected response) */
-	public resolve(uuid: string, data: unknown): boolean {
+	public resolve(uuid: string, data: unknown, peer?: number): boolean {
-		const call = this._take(uuid)
+		const call = this._take(uuid, peer)
 		call?.resolve(data)
 		return call !== undefined
 	}
 
-	/** @returns `false` if no call waits for this uuid */
+	/** @returns `false` if no call from `peer` waits for this uuid */
-	public reject(uuid: string, error: Error): boolean {
+	public reject(uuid: string, error: Error, peer?: number): boolean {
-		const call = this._take(uuid)
+		const call = this._take(uuid, peer)
 		call?.reject(error)
 		return call !== undefined
 	}
 
-	private _take(uuid: string): Call | undefined {
+	private _take(uuid: string, peer: number | undefined): Call | undefined {
 		const call = this._calls.get(uuid)
-		if (call) {
+		if (!call || (call.peer !== undefined && call.peer !== peer)) return
+
 		this._calls.delete(uuid)
 		clearTimeout(call.timer)
-		}
 		return call
 	}
 

rpc/src/utils/types.ts

@@ -48,6 +48,7 @@ export type RPCState = {
 	calledTo: RPCEnvironment
 	error: RPCErrorPayload | null
 	data: unknown[] | null
+	/** Server id of the player involved. The server fills it from `source`, never trusting the sender */
 	player: number | null
 	type: RPCEventType
 }
@@ -94,7 +95,6 @@ export enum RPCEvents {
  */
 export enum RPCErrors {
 	EVENT_NOT_REGISTERED = 'Event not registered',
-	NO_PLAYER = 'No player (failed to resolve from local index)',
 	UNKNOWN_NATIVE = 'Unknown native event (if you are sure this exists - use native handler)',
 	UNKNOWN_ENVIRONMENT = 'Unknown environment (must be either "server", "client" or "webview")',
 	TIMEOUT = 'Timed out waiting for response',