fix: enforce room membership on the client-to-client relay

packages/transfer-server/src/JsBridgeE2EEServer.ts

@@ -10,6 +10,7 @@ import type {
   IJsBridgeMessagePayload,
   IJsonRpcRequest,
 } from '@onekeyfe/cross-inpage-provider-types';
+import type { RoomManager } from './roomManager';
 import type { Socket } from 'socket.io';
 
 const logger = createModuleLogger('jsBridge');
@@ -46,11 +47,43 @@ const CLIENT_TO_CLIENT_RATE_LIMIT_ERROR_CODE = -387_155_488;
 // Rate limiting whitelist - methods that are exempt from rate limiting
 const RATE_LIMIT_WHITELIST = new Set([
   'changeTransferDirection',
-  'getRoomUsers',
   'leaveRoom',
   'cancelTransfer',
 ]);
 
+/**
+ * Per-method rate limit windows, overriding RATE_LIMIT_INTERVAL_MS.
+ *
+ * This table is for legitimate high-frequency callers that a shipped client
+ * already depends on. It is not a way to opt out of rate limiting: a method
+ * absent from here is limited at RATE_LIMIT_INTERVAL_MS, and that default is
+ * the point - a newly added method is protected without anyone having to
+ * remember to protect it.
+ *
+ * Adding an entry means stating, on that entry, which caller needs it, at what
+ * frequency, and why the default window cannot serve it. An entry is a
+ * compatibility shim and has a lifetime: remove it once its caller no longer
+ * needs it, rather than leaving a permanent hole behind.
+ *
+ * A window also has to stay meaningfully below the caller's polling interval.
+ * setInterval fixes the interval at which requests are *sent*; network latency
+ * shifts every request by roughly the same amount, so it cancels out of the gap
+ * the server observes, and only jitter moves that gap - in both directions. A
+ * 1000ms window against a 1000ms poll therefore sits exactly on the threshold
+ * rather than safely above it: measured over localhost, where latency is under
+ * a millisecond and stable, timer drift alone still pushed one poll in 30 below
+ * it and into a rejection.
+ */
+const METHOD_RATE_LIMIT_INTERVAL_MS = new Map<string, number>([
+  // CLI, once per second for the whole pairing phase
+  // (ROOM_USERS_POLL_INTERVAL_MS in transfer-receiver-adapter.ts): it had no
+  // user-joined push to wait on, so it polls to notice a peer arriving. The
+  // default 3s window would reject two of every three polls. Remove this entry
+  // once the CLI consumes the user-joined event instead - the poll and this
+  // shim go together.
+  ['getRoomUsers', 800],
+]);
+
 const SUPPORTED_MESSAGE_TYPES: ReadonlySet<string> = new Set([
   IJsBridgeMessageTypes.REQUEST,
   IJsBridgeMessageTypes.RESPONSE,
@@ -94,15 +127,21 @@ function checkBridgePayload(
 export class JsBridgeE2EEServer extends JsBridgeBase {
   constructor(
     config: IJsBridgeConfig,
-    { socketClient }: { socketClient: Socket },
+    {
+      socketClient,
+      roomManager,
+    }: { socketClient: Socket; roomManager: RoomManager },
   ) {
     super(config);
     this.socketClient = socketClient;
+    this.roomManager = roomManager;
     this.setup();
   }
 
   private socketClient: Socket;
 
+  private roomManager: RoomManager;
+
   /**
    * Rate limit state, scoped to this connection rather than kept in a
    * module-level map that lived for the lifetime of the process.
@@ -257,8 +296,10 @@ export class JsBridgeE2EEServer extends JsBridgeBase {
 
     const now = Date.now();
     const lastTime = this.rateLimitState.get(rateLimitKey);
+    const interval =
+      METHOD_RATE_LIMIT_INTERVAL_MS.get(method) ?? RATE_LIMIT_INTERVAL_MS;
 
-    if (lastTime !== undefined && now - lastTime < RATE_LIMIT_INTERVAL_MS) {
+    if (lastTime !== undefined && now - lastTime < interval) {
       sendErrorResponse();
       return true;
     }
@@ -298,7 +339,14 @@ export class JsBridgeE2EEServer extends JsBridgeBase {
    */
   private pruneRateLimitState(now: number): void {
     for (const [key, time] of this.rateLimitState) {
-      if (now - time >= RATE_LIMIT_INTERVAL_MS) {
+      // Expire each entry against its own window, not the default one: a
+      // per-method window longer than the default would otherwise be dropped
+      // while still live, which is exactly the rate limit reset this function
+      // is written to prevent.
+      const method = key.slice(key.indexOf(':') + 1);
+      const interval =
+        METHOD_RATE_LIMIT_INTERVAL_MS.get(method) ?? RATE_LIMIT_INTERVAL_MS;
+      if (now - time >= interval) {
         this.rateLimitState.delete(key);
       }
     }
@@ -430,6 +478,16 @@ export class JsBridgeE2EEServer extends JsBridgeBase {
       return undefined;
     }
 
+    // `socket.to(roomId)` is a delivery operator: it reads the membership of the
+    // recipients and never checks the sender's. Without this, any connected
+    // socket that knows a roomId can inject client-to-client calls into a room
+    // it never joined - bypassing the room-slot invariant the pairing flow
+    // relies on. Membership is authoritative in RoomManager, so ask it.
+    if (!this.roomManager.isUserInRoom(roomId, this.socketClient.id).isInRoom) {
+      this.logInvalidPayload(eventName, payload, 'sender is not a room member');
+      return undefined;
+    }
+
     return { payload: checked.payload, roomId };
   }
 }

packages/transfer-server/src/e2eeServerApi.ts

@@ -70,6 +70,7 @@ function createBridgeE2EEServer({
     },
     {
       socketClient,
+      roomManager,
     },
   );
 }

packages/transfer-server/src/roomManager.ts

@@ -46,6 +46,17 @@ export class RoomManager {
   /**
    * Create new room
    * @returns Room information (room ID and encryption key)
+   *
+   * NOTE on `encryptionKey` (returned here and as `roomKey` from joinRoom):
+   * this key is NOT used by the OneKey client, and it is not what protects
+   * transferred data. The client derives its own end-to-end key locally from
+   * material this server never sees - the pairing code shown in the QR code
+   * (only its roomId prefix ever reaches the server), an ECDHE shared secret
+   * negotiated directly between the two devices, and the room's user list.
+   * A malicious or compromised server therefore still cannot decrypt anything,
+   * because it holds none of that material. This server never encrypts or
+   * decrypts payloads with this key either - it only relays them. The field is
+   * kept for wire compatibility with existing clients.
    */
   @e2eeApiMethod()
   async createRoom(): Promise<{ roomId: string; encryptionKey: string }> {
@@ -101,6 +112,11 @@ export class RoomManager {
    * @param encryptionKey Encryption key
    * @param socketId User's Socket ID
    * @returns Join result
+   *
+   * The returned `roomKey` is server-generated and unused by the client - see
+   * the note on createRoom(). It is not part of the end-to-end encryption
+   * scheme; the client derives its own key from the pairing code and an ECDHE
+   * exchange this server is not party to.
    */
   @e2eeApiMethod()
   async joinRoom(
@@ -186,6 +202,20 @@ export class RoomManager {
 
     await context?.socketClient.join(roomId);
 
+    // Tell the members already in the room that someone joined. The server
+    // never pushed this before, so a peer that needed to know had to poll
+    // getRoomUsers instead - which is why that method carries a high call rate.
+    //
+    // Emitted from the joining socket rather than the server, so the joiner is
+    // excluded (it does not need to be told about itself), and only after
+    // join() resolves, so the membership the event describes is already in
+    // effect if a receiver immediately calls getRoomUsers.
+    context?.socketClient.to(roomId).emit("user-joined", {
+      roomId,
+      userId,
+      userCount: room.users.size,
+    });
+
     return {
       success: true,
       userId,
@@ -310,18 +340,26 @@ export class RoomManager {
       );
     }
     logger.debug({ roomId }, "room.getRoomUsers");
-    const room = this.rooms.get(roomId);
-    if (!room) {
-      logger.debug({ roomId }, "room.getRoomUsersNotFound");
-      return [];
-    }
-    // Validate that the socket is in the room
+
+    // A room the caller cannot see must be indistinguishable from a room that
+    // does not exist. Returning [] for a missing room while throwing for a room
+    // the caller is not in turned this into a room existence oracle - and this
+    // method is exempt from rate limiting, so it could be probed at line speed.
+    // isUserInRoom() already reports false for a missing room, so both cases
+    // fail here identically, with the same error code and message.
     const socketValidation = this.isUserInRoom(roomId, context.socketClient.id);
     if (!socketValidation.isInRoom) {
-      throw new E2eeError(
-        E2eeErrorCode.SOCKET_NOT_IN_ROOM,
-        "Socket must be in the room to set transfer direction"
+      logger.debug(
+        { roomId, roomExists: this.rooms.has(roomId) },
+        "room.getRoomUsersDenied"
       );
+      throw new E2eeError(E2eeErrorCode.ROOM_NOT_FOUND, "Room not found");
+    }
+
+    // isInRoom implies the room exists.
+    const room = this.rooms.get(roomId);
+    if (!room) {
+      throw new E2eeError(E2eeErrorCode.ROOM_NOT_FOUND, "Room not found");
     }
 
     const users: IE2EESocketUserInfo[] = sortBy(

packages/transfer-server/src/types.ts

@@ -8,8 +8,19 @@ export { E2eeError, E2eeErrorCode } from './errors';
 export interface IServerToClientEvents {
   'room-created': (data: { roomId: string; encryptionKey: string }) => void;
   'room-joined': (data: { roomId: string; userId: string }) => void;
-  'user-joined': (data: { userId: string; userCount: number }) => void;
-  'user-left': (data: { userId: string; userCount: number }) => void;
+  // roomId is part of both payloads: a client multiplexes every room over one
+  // socket, so it needs to tell which room an event refers to. `user-left` has
+  // always been emitted with it - the declaration was simply out of date.
+  'user-joined': (data: {
+    roomId: string;
+    userId: string;
+    userCount: number;
+  }) => void;
+  'user-left': (data: {
+    roomId: string;
+    userId: string;
+    userCount: number;
+  }) => void;
   'encrypted-data': (data: {
     encryptedData: string;
     senderId: string;
@@ -48,6 +59,11 @@ export interface ISocketData {
 // Room data structure
 export interface IRoom {
   id: string;
+  // Server-generated key handed to clients on create/join. The OneKey client
+  // does not consume it, and this server never encrypts with it - payloads are
+  // relayed as-is. Real end-to-end protection comes from a key the clients
+  // derive themselves (pairing code + ECDHE shared secret + room user list),
+  // none of which this server holds. See RoomManager.createRoom().
   encryptionKey: string;
   users: Map<string, IE2EESocketUserInfo>;
   transferDirection?: