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.
socketio · Jun 27, 2022 · b5d0cb7 · b5d0cb7 · gitDk70 · May 24, 2023
1 parent c7514b5
commit b5d0cb7
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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$/
     );
   });
 });