fix(security): SFTP session ownership + Guacamole instruction validation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

backend/src/rdp/guacamole.service.ts

@@ -201,6 +201,39 @@ export class GuacamoleService {
     return this.encode('connect', ...values);
   }
 
+  // Opcodes that clients are allowed to send to guacd.
+  // Server-originating opcodes (img, blob, end, ack, etc.) are intentionally excluded.
+  private static ALLOWED_CLIENT_OPCODES = new Set([
+    'key', 'mouse', 'size', 'clipboard', 'sync', 'disconnect', 'nop',
+  ]);
+
+  /**
+   * Validate a raw Guacamole instruction received from a browser client.
+   *
+   * Checks:
+   *   1. The instruction matches the Guacamole wire format: <len>.<opcode>[,...]
+   *   2. The declared length matches the actual opcode length (prevents injection
+   *      via length field manipulation).
+   *   3. The opcode is in the client-allowed allowlist.
+   *
+   * Returns true if the instruction is safe to forward to guacd.
+   */
+  validateClientInstruction(instruction: string): boolean {
+    try {
+      // Must begin with a length-prefixed opcode: "<digits>.<opcode>"
+      const match = instruction.match(/^(\d+)\.([\w-]+)/);
+      if (!match) return false;
+      const declaredLen = parseInt(match[1], 10);
+      const opcode = match[2];
+      // The declared length must equal the actual opcode length to prevent
+      // smuggling a different opcode via a mismatched length prefix.
+      if (declaredLen !== opcode.length) return false;
+      return GuacamoleService.ALLOWED_CLIENT_OPCODES.has(opcode);
+    } catch {
+      return false;
+    }
+  }
+
   /**
    * Encode a Guacamole instruction.
    * Each part is length-prefixed: "<len>.<value>"

backend/src/rdp/rdp.gateway.ts

@@ -40,7 +40,20 @@ export class RdpGateway {
         if (msg.type === 'connect') {
           await this.handleConnect(client, msg);
         } else if (msg.type === 'guac') {
-          // Forward raw Guacamole instruction from browser to guacd TCP socket
+          // Validate before forwarding raw Guacamole instruction to guacd TCP socket
+          if (typeof msg.instruction !== 'string') {
+            this.send(client, { type: 'error', message: 'Invalid instruction' });
+            return;
+          }
+          if (msg.instruction.length > 65536) {
+            this.send(client, { type: 'error', message: 'Instruction too large' });
+            return;
+          }
+          if (!this.guacamole.validateClientInstruction(msg.instruction)) {
+            this.logger.warn(`[RDP] Blocked invalid Guacamole instruction: ${msg.instruction.substring(0, 80)}`);
+            this.send(client, { type: 'error', message: 'Invalid instruction' });
+            return;
+          }
           const socket = this.clientSockets.get(client);
           if (socket && !socket.destroyed) {
             socket.write(msg.instruction);

backend/src/terminal/sftp.gateway.ts

@@ -8,6 +8,9 @@ const MAX_EDIT_SIZE = 5 * 1024 * 1024; // 5MB
 export class SftpGateway {
   private readonly logger = new Logger(SftpGateway.name);
 
+  // Maps WebSocket client → set of session IDs they own
+  private clientSessions = new Map<any, Set<string>>();
+
   constructor(
     private ssh: SshConnectionService,
     private wsAuth: WsAuthGuard,
@@ -21,6 +24,9 @@ export class SftpGateway {
     }
     this.logger.log(`SFTP WS connected: ${user.email}`);
 
+    // Initialize an empty session ownership set for this client
+    this.clientSessions.set(client, new Set());
+
     client.on('message', async (raw: Buffer) => {
       try {
         const msg = JSON.parse(raw.toString());
@@ -31,6 +37,10 @@ export class SftpGateway {
         this.send(client, { type: 'error', message: err.message });
       }
     });
+
+    client.on('close', () => {
+      this.clientSessions.delete(client);
+    });
   }
 
   private async handleMessage(client: any, msg: any) {
@@ -39,6 +49,16 @@ export class SftpGateway {
       return this.send(client, { type: 'error', message: 'sessionId required' });
     }
 
+    const ownedSessions = this.clientSessions.get(client);
+
+    // If this client has already claimed at least one session, enforce ownership
+    // for all subsequent requests. A client claims a sessionId on first successful
+    // SFTP channel open (see registration below).
+    if (ownedSessions && ownedSessions.size > 0 && !ownedSessions.has(sessionId)) {
+      this.logger.warn(`[SFTP] Session access denied: client does not own sessionId=${sessionId}`);
+      return this.send(client, { type: 'error', message: 'Session access denied' });
+    }
+
     this.logger.log(`[SFTP] Getting SFTP channel for session ${sessionId}...`);
     let sftp: any;
     try {
@@ -49,6 +69,12 @@ export class SftpGateway {
       return this.send(client, { type: 'error', message: `SFTP channel failed: ${err.message}` });
     }
 
+    // Register this sessionId as owned by this client on first successful channel open
+    if (ownedSessions && !ownedSessions.has(sessionId)) {
+      ownedSessions.add(sessionId);
+      this.logger.log(`[SFTP] Registered sessionId=${sessionId} for client`);
+    }
+
     // Listen for SFTP channel errors
     sftp.on('error', (err: any) => {
       this.logger.error(`[SFTP] Channel error event: ${err.message}`);