Merge pull request #2866 from QuentinGruber/dos-fixes

DoS Fixes

Author: Z1Meme

src/servers/SoeServer/soeclient.ts

@@ -45,6 +45,7 @@ export default class SOEClient {
   outputStream: SOEOutputStream;
   soeClientId: string;
   lastKeepAliveTimer: NodeJS.Timeout | null = null;
+  sendingInterval: NodeJS.Timeout | null = null;
   isDeleted: boolean = false;
   stats: SOEClientStats = {
     totalPhysicalPacketSent: 0,
@@ -77,6 +78,14 @@ export default class SOEClient {
   }
   closeTimers() {
     clearInterval(this._statsResetTimer as unknown as number);
+    if (this.sendingInterval) {
+      clearInterval(this.sendingInterval);
+      this.sendingInterval = null;
+    }
+    if (this.lastKeepAliveTimer) {
+      clearTimeout(this.lastKeepAliveTimer);
+      this.lastKeepAliveTimer = null;
+    }
   }
   private _resetStats() {
     this.stats.totalPhysicalPacketSent = 0;

src/servers/SoeServer/soeinputstream.ts

@@ -21,6 +21,13 @@ import {
 } from "../../utils/constants";
 
 const debug = require("debug")("SOEInputStream");
+
+// Upper bound on a reassembled payload. The announced size comes straight off
+// the wire, so without this a single packet can request a multi GB allocation.
+const MAX_REASSEMBLY_SIZE = 16 * 1024 * 1024;
+// Upper bound on out of order packets buffered per client.
+const MAX_BUFFERED_PACKETS = 1024;
+
 type appData = { payload: Buffer; isFragment: boolean };
 export class SOEInputStream extends EventEmitter {
   _nextSequence: wrappedUint16 = new wrappedUint16(0);
@@ -53,8 +60,28 @@ export class SOEInputStream extends EventEmitter {
     // cpf == current processed fragment
     if (!this.has_cpf) {
       const firstPacket = this._appDataMap.get(firstPacketSequence) as appData; // should be always defined
+      if (firstPacket.payload.length < DATA_HEADER_SIZE) {
+        this._appDataMap.delete(firstPacketSequence);
+        this.emit(
+          "error",
+          new Error(
+            "processDataFragments: first fragment too small to hold a size header"
+          )
+        );
+        return [];
+      }
       // the total size is written has a uint32 at the first packet of a fragmented data
       this.cpf_totalSize = firstPacket.payload.readUInt32BE(0);
+      if (this.cpf_totalSize > MAX_REASSEMBLY_SIZE) {
+        this._appDataMap.delete(firstPacketSequence);
+        this.emit(
+          "error",
+          new Error(
+            `processDataFragments: announced size ${this.cpf_totalSize} exceeds the ${MAX_REASSEMBLY_SIZE} limit`
+          )
+        );
+        return [];
+      }
       this.cpf_dataSize = 0;
 
       this.cpf_dataWithoutHeader = Buffer.allocUnsafe(this.cpf_totalSize);
@@ -82,6 +109,7 @@ export class SOEInputStream extends EventEmitter {
         this.cpf_dataSize += fragmentDataLen;
 
         if (this.cpf_dataSize > this.cpf_totalSize) {
+          this.has_cpf = false;
           this.emit(
             "error",
             new Error(
@@ -96,6 +124,7 @@ export class SOEInputStream extends EventEmitter {
                 ")"
             )
           );
+          return [];
         }
         if (this.cpf_dataSize === this.cpf_totalSize) {
           // Delete all the processed fragments from memory
@@ -119,23 +148,21 @@ export class SOEInputStream extends EventEmitter {
   }
 
   private _processData(): void {
-    const nextFragmentSequence =
-      (this._lastProcessedSequence + 1) & MAX_SEQUENCE;
-    const dataToProcess = this._appDataMap.get(nextFragmentSequence);
-    if (dataToProcess) {
-      let appData: Array<Buffer> = [];
-
-      if (dataToProcess.isFragment) {
-        appData = this.processFragmentedData(nextFragmentSequence);
-      } else {
-        appData = this.processSingleData(dataToProcess, nextFragmentSequence);
-      }
-      if (appData.length) {
-        this.processAppData(appData);
-        // In case there is more data to process
-        // It can happen when packets are received out of order
-        this._processData();
-      }
+    // Iterative rather than recursive: a client can fill _appDataMap with out
+    // of order packets and then send the missing one, which would otherwise
+    // recurse once per buffered packet and blow the stack.
+    for (;;) {
+      const nextFragmentSequence =
+        (this._lastProcessedSequence + 1) & MAX_SEQUENCE;
+      const dataToProcess = this._appDataMap.get(nextFragmentSequence);
+      if (!dataToProcess) return;
+
+      const appData: Array<Buffer> = dataToProcess.isFragment
+        ? this.processFragmentedData(nextFragmentSequence)
+        : this.processSingleData(dataToProcess, nextFragmentSequence);
+
+      if (!appData.length) return;
+      this.processAppData(appData);
     }
   }
 
@@ -189,6 +216,13 @@ export class SOEInputStream extends EventEmitter {
       " fragment=" + isFragment + ", lastAck: " + this._lastAck.get()
     );
     if (sequence >= this._nextSequence.get()) {
+      if (
+        !this._appDataMap.has(sequence) &&
+        this._appDataMap.size >= MAX_BUFFERED_PACKETS
+      ) {
+        // Too many out of order packets buffered, drop rather than grow forever
+        return;
+      }
       this._appDataMap.set(sequence, { payload: data, isFragment: isFragment });
       const wasInOrder = this.acknowledgeInputData(sequence);
       if (wasInOrder) {

src/servers/SoeServer/soeoutputstream.ts

@@ -96,6 +96,12 @@ export class SOEOutputStream extends EventEmitter {
   }
 
   writeReliable(data: Uint8Array): void {
+    if (this._fragmentSize < 1) {
+      // Would make the fragmentation loop below never terminate
+      throw new Error(
+        `Cannot fragment data with fragment size ${this._fragmentSize}`
+      );
+    }
     if (data.length <= this._fragmentSize) {
       this._reliable_sequence.increment();
       this.addToCache(this._reliable_sequence.get(), data, false);
@@ -156,16 +162,17 @@ export class SOEOutputStream extends EventEmitter {
 
   ack(sequence: number, unAckData: Map<number, number>): void {
     const wrappedSequence = wrappedUint16.wrap(sequence);
-    const wrapThreshold = MAX_UINT16 / 2;
+    const SEQUENCE_SPACE = MAX_UINT16 + 1;
 
-    // Determine if wrappedSequence is ahead of lastAck, including wrap-around handling
-    const isSequenceAhead =
-      wrappedSequence > this.lastAck.get() ||
-      (wrappedSequence < this.lastAck.get() &&
-        this.lastAck.get() - wrappedSequence > wrapThreshold);
+    // Forward distance from lastAck to the acked sequence, in sequence space.
+    const distance =
+      (wrappedSequence - this.lastAck.get() + SEQUENCE_SPACE) % SEQUENCE_SPACE;
 
-    // If the sequence is ahead, delete all cached data/timers up to the given ack sequence
-    if (isSequenceAhead) {
+    // We never have more than maxSequenceAvailable packets in flight, so an ack
+    // further ahead than that is bogus. Without this bound a single crafted ack
+    // costs up to 65535 cache lookups, and a sequence that wrap() cannot bring
+    // back into range would loop forever.
+    if (distance > 0 && distance <= this.maxSequenceAvailable) {
       while (this.lastAck.get() !== wrappedSequence) {
         this.removeFromCache(this.lastAck.get());
         unAckData.delete(this.lastAck.get());
@@ -176,7 +183,7 @@ export class SOEOutputStream extends EventEmitter {
         }
       }
     } else {
-      // If an out-of-order ack is received, delete that specific entry if it exists
+      // Out of order or bogus ack, delete that specific entry if it exists
       if (unAckData.has(wrappedSequence)) {
         this.removeFromCache(wrappedSequence);
         unAckData.delete(wrappedSequence);
@@ -217,6 +224,9 @@ export class SOEOutputStream extends EventEmitter {
   }
 
   setFragmentSize(value: number): void {
+    if (!Number.isInteger(value) || value < 1) {
+      throw new Error(`Invalid fragment size: ${value}`);
+    }
     this._fragmentSize = value;
   }
 }

src/servers/SoeServer/soeserver.ts

@@ -26,6 +26,8 @@ const debug = require("debug")("SOEServer");
 
 const MAX_PACKETS_PER_IP_PER_SEC = 500;
 const MAX_SESSIONS_PER_IP_PER_MIN = 5;
+const MIN_UDP_LENGTH = 512;
+const MAX_UDP_LENGTH = 65535;
 
 interface RateWindow {
   count: number;
@@ -279,8 +281,19 @@ export class SOEServer extends EventEmitter {
 
   private _createClient(remote: RemoteInfo) {
     const client = new SOEClient(remote, this._crcSeed, this._cryptoKey);
+    // Armed here rather than in the SessionRequest handler: a client that never
+    // completes a session request must still expire. We delete it ourselves
+    // because the gateway/zone disconnect path can only find clients by
+    // sessionId, which a half-open client never sets.
+    client.lastKeepAliveTimer = this.keepAliveTimeoutTime
+      ? setTimeout(() => {
+          debug("Client keep alive timeout");
+          this.deleteClient(client);
+          this.emit("disconnect", client);
+        }, this.keepAliveTimeoutTime)
+      : null;
     // Start a persistent sending loop for this client
-    (client as any).sendingInterval = setInterval(() => {
+    client.sendingInterval = setInterval(() => {
       this.sendingProcess(client);
     }, this._waitTimeMs);
     client.inputStream.on("appdata", (data: Buffer) => {
@@ -329,7 +342,12 @@ export class SOEServer extends EventEmitter {
           "Received session request from " + client.address + ":" + client.port
         );
         client.sessionId = packet.session_id;
-        client.clientUdpLength = packet.udp_length;
+        // udp_length is attacker controlled. An unclamped value makes the
+        // derived fragment size <= 0, which hangs writeReliable() forever.
+        client.clientUdpLength = Math.min(
+          Math.max(packet.udp_length >>> 0, MIN_UDP_LENGTH),
+          MAX_UDP_LENGTH
+        );
         client.protocolName = packet.protocol;
         client.serverUdpLength = this._udpLength;
         client.crcSeed = this._crcSeed;
@@ -340,14 +358,6 @@ export class SOEServer extends EventEmitter {
         client.outputStream.setFragmentSize(
           client.clientUdpLength - (4 + this._crcLength)
         );
-        // setup the keep alive timer
-        client.lastKeepAliveTimer = this.keepAliveTimeoutTime
-          ? setTimeout(() => {
-              debug("Client keep alive timeout");
-              this.emit("disconnect", client);
-            }, this.keepAliveTimeoutTime)
-          : null;
-
         const sessionReply = this.createLogicalPacket(SoeOpcode.SessionReply, {
           session_id: client.sessionId,
           crc_seed: client.crcSeed,
@@ -467,37 +477,15 @@ export class SOEServer extends EventEmitter {
     try {
       if (!this._checkIpRateLimit(remote.address)) return;
 
-      let client: SOEClient;
       const clientId = SOEClient.getClientId(remote);
       debug(data.length + " bytes from ", clientId);
-      // if doesn't know the client
-      if (!this._clients.has(clientId)) {
-        // if it's not a session request then we ignore it
-        if (data[1] !== 1) {
-          return;
-        }
-        if (!this._checkSessionRateLimit(remote.address)) return;
-        client = this._createClient(remote);
-      } else {
-        client = this._clients.get(clientId) as SOEClient;
-      }
-      if (data[0] === 0x00) {
-        const raw_parsed_data: string = this._protocol.parse(data);
-        if (raw_parsed_data) {
-          const parsed_data = JSON.parse(raw_parsed_data);
-          if (parsed_data.name === "Error") {
-            debug("parsing error " + parsed_data.error);
-            debug(parsed_data);
-          } else {
-            if (client.lastKeepAliveTimer) {
-              client.lastKeepAliveTimer.refresh();
-            }
-            this.handlePacket(client, parsed_data);
-          }
-        } else {
-          debug("Unmanaged packet from client", clientId, data);
-        }
-      } else {
+
+      let client = this._clients.get(clientId);
+
+      // Non-SOE data is only ever valid for an established client and must
+      // never be able to create one.
+      if (data[0] !== 0x00) {
+        if (!client) return;
         if (this._allowRawDataReception) {
           debug("Raw data received from client", clientId, data);
           this.emit("appdata", client, data, true); // Unreliable + Unordered
@@ -508,10 +496,46 @@ export class SOEServer extends EventEmitter {
             data
           );
         }
+        return;
       }
+
+      // Cheap pre-filter so a junk flood never reaches the protocol parser
+      if (!client && data[1] !== 1) return;
+
+      const raw_parsed_data: string = this._protocol.parse(data);
+      if (!raw_parsed_data) {
+        debug("Unmanaged packet from client", clientId, data);
+        return;
+      }
+
+      let parsed_data;
+      try {
+        parsed_data = JSON.parse(raw_parsed_data);
+      } catch {
+        // h1emu-core can emit malformed JSON for hostile input, drop the packet
+        debug("Failed to parse packet JSON from " + clientId);
+        return;
+      }
+
+      if (parsed_data.name === "Error") {
+        debug("parsing error " + parsed_data.error);
+        debug(parsed_data);
+        return;
+      }
+
+      if (!client) {
+        // An unknown client may only ever open a session, nothing else.
+        if (parsed_data.name !== "SessionRequest") return;
+        if (!this._checkSessionRateLimit(remote.address)) return;
+        client = this._createClient(remote);
+      }
+
+      if (client.lastKeepAliveTimer) {
+        client.lastKeepAliveTimer.refresh();
+      }
+      this.handlePacket(client, parsed_data);
     } catch (e) {
       console.log(e);
-      process.exitCode = 1;
     }
   }
 
@@ -523,15 +547,17 @@ export class SOEServer extends EventEmitter {
     if (udpLength !== undefined) {
       this._udpLength = udpLength;
     }
-    setInterval(() => {
+    // Swept at a cadence matching the entry TTLs, otherwise a spoofed source
+    // flood grows these maps unbounded between sweeps.
+    this._packetResetInterval = setInterval(() => {
       const now = Date.now();
       for (const [ip, entry] of this._ipRateMap) {
         if (now - entry.windowStart > 2000) this._ipRateMap.delete(ip);
       }
       for (const [ip, entry] of this._sessionRequestMap) {
         if (now - entry.windowStart > 62000) this._sessionRequestMap.delete(ip);
       }
-    }, 60000);
+    }, 5000);
 
     this._connection.on("message", (data, remote) => {
       this.onMessage(data, remote);
@@ -812,10 +838,7 @@ export class SOEServer extends EventEmitter {
 
   deleteClient(client: SOEClient): void {
     client.isDeleted = true;
-    if ((client as any).sendingInterval) {
-      clearInterval((client as any).sendingInterval);
-      (client as any).sendingInterval = null;
-    }
+    client.closeTimers();
     this._clients.delete(client.soeClientId);
     debug("client connection from port : ", client.port, " deleted");
   }