Skip to content

Commit b559f05

Browse files
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 b5d0cb7
1 parent af1b23c commit b559f05

File tree

4 files changed

+66
-3
lines changed

4 files changed

+66
-3
lines changed

lib/binary.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,16 @@ export function reconstructPacket(packet, buffers) {
6060
function _reconstructPacket(data, buffers) {
6161
if (!data) return data;
6262

63-
if (data && data._placeholder) {
64-
return buffers[data.num]; // appropriate buffer (should be natural order anyway)
63+
if (data && data._placeholder === true) {
64+
const isIndexValid =
65+
typeof data.num === "number" &&
66+
data.num >= 0 &&
67+
data.num < buffers.length;
68+
if (isIndexValid) {
69+
return buffers[data.num]; // appropriate buffer (should be natural order anyway)
70+
} else {
71+
throw new Error("illegal attachments");
72+
}
6573
} else if (Array.isArray(data)) {
6674
for (let i = 0; i < data.length; i++) {
6775
data[i] = _reconstructPacket(data[i], buffers);

lib/index.ts

+3
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ export class Decoder extends Emitter {
129129
public add(obj: any) {
130130
let packet;
131131
if (typeof obj === "string") {
132+
if (this.reconstructor) {
133+
throw new Error("got plaintext data when reconstructing a packet");
134+
}
132135
packet = this.decodeString(obj);
133136
if (
134137
packet.type === PacketType.BINARY_EVENT ||

test/buffer.js

+49-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
const { PacketType } = require("..");
1+
const { PacketType, Decoder } = require("../");
22
const helpers = require("./helpers.js");
3+
const expect = require("expect.js");
34

45
describe("parser", () => {
56
it("encodes a Buffer", (done) => {
@@ -14,6 +15,18 @@ describe("parser", () => {
1415
);
1516
});
1617

18+
it("encodes a nested Buffer", (done) => {
19+
helpers.test_bin(
20+
{
21+
type: PacketType.EVENT,
22+
data: ["a", { b: ["c", Buffer.from("abc", "utf8")] }],
23+
id: 23,
24+
nsp: "/cool",
25+
},
26+
done
27+
);
28+
});
29+
1730
it("encodes a binary ack with Buffer", (done) => {
1831
helpers.test_bin(
1932
{
@@ -25,4 +38,39 @@ describe("parser", () => {
2538
done
2639
);
2740
});
41+
42+
it("throws an error when adding an attachment with an invalid 'num' attribute (string)", () => {
43+
const decoder = new Decoder();
44+
45+
expect(() => {
46+
decoder.add('51-["hello",{"_placeholder":true,"num":"splice"}]');
47+
decoder.add(Buffer.from("world"));
48+
}).to.throwException(/^illegal attachments$/);
49+
});
50+
51+
it("throws an error when adding an attachment with an invalid 'num' attribute (out-of-bound)", () => {
52+
const decoder = new Decoder();
53+
54+
expect(() => {
55+
decoder.add('51-["hello",{"_placeholder":true,"num":1}]');
56+
decoder.add(Buffer.from("world"));
57+
}).to.throwException(/^illegal attachments$/);
58+
});
59+
60+
it("throws an error when adding an attachment without header", () => {
61+
const decoder = new Decoder();
62+
63+
expect(() => {
64+
decoder.add(Buffer.from("world"));
65+
}).to.throwException(/^got binary data when not reconstructing a packet$/);
66+
});
67+
68+
it("throws an error when decoding a binary event without attachments", () => {
69+
const decoder = new Decoder();
70+
71+
expect(() => {
72+
decoder.add('51-["hello",{"_placeholder":true,"num":0}]');
73+
decoder.add('2["hello"]');
74+
}).to.throwException(/^got plaintext data when reconstructing a packet$/);
75+
});
2876
});

test/parser.js

+4
Original file line numberDiff line numberDiff line change
@@ -146,5 +146,9 @@ describe("parser", () => {
146146
expect(() => new Decoder().add("999")).to.throwException(
147147
/^unknown packet type 9$/
148148
);
149+
150+
expect(() => new Decoder().add(999)).to.throwException(
151+
/^Unknown type: 999$/
152+
);
149153
});
150154
});

0 commit comments

Comments
 (0)