Skip to content

Commit

Permalink
fix: check the format of the index of each attachment
Browse files Browse the repository at this point in the history
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
  • Loading branch information
darrachequesne committed Nov 9, 2022
1 parent 3b0a392 commit fb21e42
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 4 deletions.
12 changes: 10 additions & 2 deletions binary.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,16 @@ exports.reconstructPacket = function(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) {
var 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 (isArray(data)) {
for (var i = 0; i < data.length; i++) {
data[i] = _reconstructPacket(data[i], buffers);
Expand Down
3 changes: 3 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ Emitter(Decoder.prototype);
Decoder.prototype.add = function(obj) {
var packet;
if (typeof obj === 'string') {
if (this.reconstructor) {
throw new Error("got plaintext data when reconstructing a packet");
}
packet = decodeString(obj);
if (exports.BINARY_EVENT === packet.type || exports.BINARY_ACK === packet.type) { // binary packet's json
this.reconstructor = new BinaryReconstructor(packet);
Expand Down
47 changes: 45 additions & 2 deletions test/buffer.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
var parser = require('../index.js');
var expect = require('expect.js');
var helpers = require('./helpers.js');
var encode = parser.encode;
var decode = parser.decode;
var Decoder = parser.Decoder;

describe('parser', function() {
it('encodes a Buffer', function() {
Expand All @@ -14,6 +13,15 @@ describe('parser', function() {
});
});

it("encodes a nested Buffer", function() {
helpers.test_bin({
type: parser.BINARY_EVENT,
data: ["a", { b: ["c", Buffer.from("abc", "utf8")] }],
id: 23,
nsp: "/cool",
});
});

it('encodes a binary ack with Buffer', function() {
helpers.test_bin({
type: parser.BINARY_ACK,
Expand All @@ -22,4 +30,39 @@ describe('parser', function() {
nsp: '/back'
})
});

it("throws an error when adding an attachment with an invalid 'num' attribute (string)", function() {
var decoder = new Decoder();

expect(function() {
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)", function() {
var decoder = new Decoder();

expect(function() {
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", function() {
var decoder = new Decoder();

expect(function() {
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", function() {
var decoder = new Decoder();

expect(function() {
decoder.add('51-["hello",{"_placeholder":true,"num":0}]');
decoder.add('2["hello"]');
}).to.throwException(/^got plaintext data when reconstructing a packet$/);
});
});

0 comments on commit fb21e42

Please # to comment.