From b5d0cb7dc56a0601a09b056beaeeb0e43b160050 Mon Sep 17 00:00:00 2001
From: Damien Arrachequesne <damien.arrachequesne@gmail.com>
Date: Mon, 27 Jun 2022 15:35:31 +0200
Subject: [PATCH] fix: check the format of the index of each attachment

A specially crafted packet could be incorrectly decoded.

Example:

```js
const decoder = new Decoder();

decoder.on("decoded", (packet) => {
  console.log(packet.data); // prints [ 'hello', [Function: splice] ]
})

decoder.add('51-["hello",{"_placeholder":true,"num":"splice"}]');
decoder.add(Buffer.from("world"));
```

As usual, please remember not to trust user input.
---
 lib/binary.ts  | 12 ++++++++++--
 lib/index.ts   |  3 +++
 test/buffer.js | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 test/parser.js |  4 ++++
 4 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/lib/binary.ts b/lib/binary.ts
index 65d9789..3bc41b4 100644
--- a/lib/binary.ts
+++ b/lib/binary.ts
@@ -60,8 +60,16 @@ export function reconstructPacket(packet, buffers) {
 function _reconstructPacket(data, buffers) {
   if (!data) return data;
 
-  if (data && data._placeholder) {
-    return buffers[data.num]; // appropriate buffer (should be natural order anyway)
+  if (data && data._placeholder === true) {
+    const isIndexValid =
+      typeof data.num === "number" &&
+      data.num >= 0 &&
+      data.num < buffers.length;
+    if (isIndexValid) {
+      return buffers[data.num]; // appropriate buffer (should be natural order anyway)
+    } else {
+      throw new Error("illegal attachments");
+    }
   } else if (Array.isArray(data)) {
     for (let i = 0; i < data.length; i++) {
       data[i] = _reconstructPacket(data[i], buffers);
diff --git a/lib/index.ts b/lib/index.ts
index 453425f..1170d17 100644
--- a/lib/index.ts
+++ b/lib/index.ts
@@ -145,6 +145,9 @@ export class Decoder extends Emitter<{}, {}, DecoderReservedEvents> {
   public add(obj: any) {
     let packet;
     if (typeof obj === "string") {
+      if (this.reconstructor) {
+        throw new Error("got plaintext data when reconstructing a packet");
+      }
       packet = this.decodeString(obj);
       if (
         packet.type === PacketType.BINARY_EVENT ||
diff --git a/test/buffer.js b/test/buffer.js
index af8e1ba..367ebcd 100644
--- a/test/buffer.js
+++ b/test/buffer.js
@@ -1,5 +1,6 @@
-const { PacketType } = require("..");
+const { PacketType, Decoder } = require("../");
 const helpers = require("./helpers.js");
+const expect = require("expect.js");
 
 describe("parser", () => {
   it("encodes a Buffer", (done) => {
@@ -14,6 +15,18 @@ describe("parser", () => {
     );
   });
 
+  it("encodes a nested Buffer", (done) => {
+    helpers.test_bin(
+      {
+        type: PacketType.EVENT,
+        data: ["a", { b: ["c", Buffer.from("abc", "utf8")] }],
+        id: 23,
+        nsp: "/cool",
+      },
+      done
+    );
+  });
+
   it("encodes a binary ack with Buffer", (done) => {
     helpers.test_bin(
       {
@@ -25,4 +38,39 @@ describe("parser", () => {
       done
     );
   });
+
+  it("throws an error when adding an attachment with an invalid 'num' attribute (string)", () => {
+    const decoder = new Decoder();
+
+    expect(() => {
+      decoder.add('51-["hello",{"_placeholder":true,"num":"splice"}]');
+      decoder.add(Buffer.from("world"));
+    }).to.throwException(/^illegal attachments$/);
+  });
+
+  it("throws an error when adding an attachment with an invalid 'num' attribute (out-of-bound)", () => {
+    const decoder = new Decoder();
+
+    expect(() => {
+      decoder.add('51-["hello",{"_placeholder":true,"num":1}]');
+      decoder.add(Buffer.from("world"));
+    }).to.throwException(/^illegal attachments$/);
+  });
+
+  it("throws an error when adding an attachment without header", () => {
+    const decoder = new Decoder();
+
+    expect(() => {
+      decoder.add(Buffer.from("world"));
+    }).to.throwException(/^got binary data when not reconstructing a packet$/);
+  });
+
+  it("throws an error when decoding a binary event without attachments", () => {
+    const decoder = new Decoder();
+
+    expect(() => {
+      decoder.add('51-["hello",{"_placeholder":true,"num":0}]');
+      decoder.add('2["hello"]');
+    }).to.throwException(/^got plaintext data when reconstructing a packet$/);
+  });
 });
diff --git a/test/parser.js b/test/parser.js
index 426e77c..4fdd938 100644
--- a/test/parser.js
+++ b/test/parser.js
@@ -146,5 +146,9 @@ describe("parser", () => {
     expect(() => new Decoder().add("999")).to.throwException(
       /^unknown packet type 9$/
     );
+
+    expect(() => new Decoder().add(999)).to.throwException(
+      /^Unknown type: 999$/
+    );
   });
 });