From b559f050ee02bd90bd853b9823f8de7fa94a80d4 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne 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. Backported from https://github.com/socketio/socket.io-parser/commit/b5d0cb7dc56a0601a09b056beaeeb0e43b160050 --- 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 89956c2..da8de0d 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 636df23..93a1421 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -129,6 +129,9 @@ export class Decoder extends Emitter { 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$/ + ); }); });