Skip to content

Commit a76e211

Browse files
Nadav0077lpinca
Nadav0077
authored and
lpinca
committed
[security] Limit retained message parts
Previously, the receiver could retain one `Buffer` entry per buffered chunk or message fragment until enough data was parsed or the message completed. A peer could use many tiny fragments/chunks and make retained memory scale with retained part count rather than message payload size. Add configurable `maxBufferedChunks` and `maxFragments` options to bound the number of retained parts. When either limit is exceeded, emit a `WS_ERR_TOO_MANY_BUFFERED_PARTS` error and close the connection with close code 1008. Signed-off-by: Nadav0077 <18245584+Nadav0077@users.noreply.github.com>
1 parent d87f3b6 commit a76e211

7 files changed

Lines changed: 178 additions & 7 deletions

File tree

‎doc/ws.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ This class represents a WebSocket server. It extends the `EventEmitter`.
6363
- `noServer` {Boolean} Enable no server mode.
6464
- `clientTracking` {Boolean} Specifies whether or not to track clients.
6565
- `perMessageDeflate` {Boolean|Object} Enable/disable permessage-deflate.
66+
- `maxBufferedChunks` {Number} The maximum number of buffered data chunks.
67+
Defaults to 1048576. Set to 0 to disable the limit.
68+
- `maxFragments` {Number} The maximum number of fragments in a message.
69+
Defaults to 131072. Set to 0 to disable the limit.
6670
- `maxPayload` {Number} The maximum allowed message size in bytes.
6771
- `callback` {Function}
6872

@@ -247,6 +251,10 @@ This class represents a WebSocket. It extends the `EventEmitter`.
247251
- `protocolVersion` {Number} Value of the `Sec-WebSocket-Version` header.
248252
- `origin` {String} Value of the `Origin` or `Sec-WebSocket-Origin` header
249253
depending on the `protocolVersion`.
254+
- `maxBufferedChunks` {Number} The maximum number of buffered data chunks.
255+
Defaults to 1048576. Set to 0 to disable the limit.
256+
- `maxFragments` {Number} The maximum number of fragments in a message.
257+
Defaults to 131072. Set to 0 to disable the limit.
250258
- `maxPayload` {Number} The maximum allowed message size in bytes.
251259
- Any other option allowed in [http.request()][] or [https.request()][].
252260

‎lib/receiver.js‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,25 @@ class Receiver extends Writable {
3131
* @param {String} binaryType The type for binary data
3232
* @param {Object} extensions An object containing the negotiated extensions
3333
* @param {Number} maxPayload The maximum allowed message length
34+
* @param {Number} [maxBufferedChunks=0] The maximum number of
35+
* buffered data chunks
36+
* @param {Number} [maxFragments=0] The maximum number of message
37+
* fragments
3438
*/
35-
constructor(binaryType, extensions, maxPayload) {
39+
constructor(
40+
binaryType,
41+
extensions,
42+
maxPayload,
43+
maxBufferedChunks,
44+
maxFragments
45+
) {
3646
super();
3747

3848
this._binaryType = binaryType || BINARY_TYPES[0];
3949
this[kWebSocket] = undefined;
4050
this._extensions = extensions || {};
51+
this._maxBufferedChunks = maxBufferedChunks | 0;
52+
this._maxFragments = maxFragments | 0;
4153
this._maxPayload = maxPayload | 0;
4254

4355
this._bufferedBytes = 0;
@@ -69,6 +81,13 @@ class Receiver extends Writable {
6981
_write(chunk, encoding, cb) {
7082
if (this._opcode === 0x08 && this._state == GET_INFO) return cb();
7183

84+
if (
85+
this._maxBufferedChunks > 0 &&
86+
this._buffers.length >= this._maxBufferedChunks
87+
) {
88+
return cb(error(RangeError, 'Too many buffered chunks', false, 1008));
89+
}
90+
7291
this._bufferedBytes += chunk.length;
7392
this._buffers.push(chunk);
7493
this.startLoop(cb);
@@ -341,6 +360,14 @@ class Receiver extends Writable {
341360
}
342361

343362
if (data.length) {
363+
if (
364+
this._maxFragments > 0 &&
365+
this._fragments.length >= this._maxFragments
366+
) {
367+
this._loop = false;
368+
return error(RangeError, 'Too many message fragments', false, 1008);
369+
}
370+
344371
//
345372
// This message is not compressed so its lenght is the sum of the payload
346373
// length of all fragments.
@@ -373,6 +400,15 @@ class Receiver extends Writable {
373400
);
374401
}
375402

403+
if (
404+
this._maxFragments > 0 &&
405+
this._fragments.length >= this._maxFragments
406+
) {
407+
return cb(
408+
error(RangeError, 'Too many message fragments', false, 1008)
409+
);
410+
}
411+
376412
this._fragments.push(buf);
377413
}
378414

‎lib/websocket-server.js‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ class WebSocketServer extends EventEmitter {
2727
* clients
2828
* @param {Function} options.handleProtocols An hook to handle protocols
2929
* @param {String} options.host The hostname where to bind the server
30+
* @param {Number} [options.maxBufferedChunks=1048576] The maximum number of
31+
* buffered data chunks
32+
* @param {Number} [options.maxFragments=131072] The maximum number of message
33+
* fragments
3034
* @param {Number} options.maxPayload The maximum allowed message size
3135
* @param {Boolean} options.noServer Enable no server mode
3236
* @param {String} options.path Accept only connections matching this path
@@ -42,6 +46,8 @@ class WebSocketServer extends EventEmitter {
4246

4347
options = Object.assign(
4448
{
49+
maxBufferedChunks: 1024 * 1024,
50+
maxFragments: 128 * 1024,
4551
maxPayload: 100 * 1024 * 1024,
4652
perMessageDeflate: false,
4753
handleProtocols: null,
@@ -316,7 +322,13 @@ class WebSocketServer extends EventEmitter {
316322
socket.write(headers.concat('\r\n').join('\r\n'));
317323
socket.removeListener('error', socketOnError);
318324

319-
ws.setSocket(socket, head, this.options.maxPayload);
325+
ws.setSocket(
326+
socket,
327+
head,
328+
this.options.maxPayload,
329+
this.options.maxBufferedChunks,
330+
this.options.maxFragments
331+
);
320332

321333
if (this.clients) {
322334
this.clients.add(ws);

‎lib/websocket.js‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,19 @@ class WebSocket extends EventEmitter {
133133
* @param {net.Socket} socket The network socket between the server and client
134134
* @param {Buffer} head The first packet of the upgraded stream
135135
* @param {Number} maxPayload The maximum allowed message size
136+
* @param {Number} [maxBufferedChunks=0] The maximum number of
137+
* buffered data chunks
138+
* @param {Number} [maxFragments=0] The maximum number of message
139+
* fragments
136140
* @private
137141
*/
138-
setSocket(socket, head, maxPayload) {
142+
setSocket(socket, head, maxPayload, maxBufferedChunks, maxFragments) {
139143
const receiver = new Receiver(
140144
this._binaryType,
141145
this._extensions,
142-
maxPayload
146+
maxPayload,
147+
maxBufferedChunks,
148+
maxFragments
143149
);
144150

145151
this._sender = new Sender(socket, this._extensions);
@@ -436,6 +442,10 @@ module.exports = WebSocket;
436442
* header
437443
* @param {String} options.origin Value of the `Origin` or
438444
* `Sec-WebSocket-Origin` header
445+
* @param {Number} [options.maxBufferedChunks=1048576] The maximum number of
446+
* buffered data chunks
447+
* @param {Number} [options.maxFragments=131072] The maximum number of message
448+
* fragments
439449
* @param {Number} options.maxPayload The maximum allowed message size
440450
* @param {Boolean} options.followRedirects Whether or not to follow redirects
441451
* @param {Number} options.maxRedirects The maximum number of redirects allowed
@@ -445,6 +455,8 @@ function initAsClient(websocket, address, protocols, options) {
445455
const opts = Object.assign(
446456
{
447457
protocolVersion: protocolVersions[1],
458+
maxBufferedChunks: 1024 * 1024,
459+
maxFragments: 128 * 1024,
448460
maxPayload: 100 * 1024 * 1024,
449461
perMessageDeflate: true,
450462
followRedirects: false,
@@ -662,7 +674,13 @@ function initAsClient(websocket, address, protocols, options) {
662674
}
663675
}
664676

665-
websocket.setSocket(socket, head, opts.maxPayload);
677+
websocket.setSocket(
678+
socket,
679+
head,
680+
opts.maxPayload,
681+
opts.maxBufferedChunks,
682+
opts.maxFragments
683+
);
666684
});
667685
}
668686

‎test/receiver.test.js‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,85 @@ describe('Receiver', () => {
889889
});
890890
});
891891

892+
it('emits an error if there are too many message fragments (1/2)', (done) => {
893+
const receiver = new Receiver(undefined, {}, 0, 0, 2);
894+
895+
receiver.on('error', (err) => {
896+
assert.ok(err instanceof RangeError);
897+
assert.strictEqual(err.message, 'Too many message fragments');
898+
assert.strictEqual(err[kStatusCode], 1008);
899+
done();
900+
});
901+
902+
receiver.write(
903+
Buffer.from([
904+
0x02,
905+
0x01,
906+
0x61, // First non-final binary fragment.
907+
0x00,
908+
0x01,
909+
0x62, // Continuation fragment.
910+
0x00,
911+
0x01,
912+
0x63 // Continuation fragment that exceeds the limit.
913+
])
914+
);
915+
});
916+
917+
it('emits an error if there are too many message fragments (2/2)', (done) => {
918+
const perMessageDeflate = new PerMessageDeflate();
919+
perMessageDeflate.accept([{}]);
920+
921+
const receiver = new Receiver(
922+
undefined,
923+
{
924+
'permessage-deflate': perMessageDeflate
925+
},
926+
0,
927+
0,
928+
1
929+
);
930+
const fragment1 = Buffer.from('foo');
931+
const fragment2 = Buffer.from('bar');
932+
933+
receiver.on('error', (err) => {
934+
assert.ok(err instanceof RangeError);
935+
assert.strictEqual(err.message, 'Too many message fragments');
936+
assert.strictEqual(err[kStatusCode], 1008);
937+
done();
938+
});
939+
940+
perMessageDeflate.compress(fragment1, false, (err, data) => {
941+
if (err) return done(err);
942+
943+
receiver.write(Buffer.from([0x41, data.length]));
944+
receiver.write(data);
945+
946+
perMessageDeflate.compress(fragment2, true, (err, data) => {
947+
if (err) return done(err);
948+
949+
receiver.write(Buffer.from([0x80, data.length]));
950+
receiver.write(data);
951+
});
952+
});
953+
});
954+
955+
it('emits an error if there are too many buffered chunks', (done) => {
956+
const receiver = new Receiver(undefined, {}, 0, 2);
957+
958+
receiver.on('error', (err) => {
959+
assert.ok(err instanceof RangeError);
960+
assert.strictEqual(err.message, 'Too many buffered chunks');
961+
assert.strictEqual(err[kStatusCode], 1008);
962+
done();
963+
});
964+
965+
receiver.write(Buffer.from([0x82, 0x05]));
966+
receiver.write(Buffer.from([0x61]));
967+
receiver.write(Buffer.from([0x62]));
968+
receiver.write(Buffer.from([0x63]));
969+
});
970+
892971
it("honors the 'nodebuffer' binary type", (done) => {
893972
const receiver = new Receiver();
894973
const frags = [

‎test/websocket-server.test.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,15 @@ describe('WebSocketServer', () => {
2929
});
3030
});
3131

32-
it('accepts the `maxPayload` option', (done) => {
32+
it('accepts the receiver limit options', (done) => {
33+
const maxBufferedChunks = 1024;
34+
const maxFragments = 512;
3335
const maxPayload = 20480;
3436
const wss = new WebSocket.Server(
3537
{
3638
perMessageDeflate: true,
39+
maxBufferedChunks,
40+
maxFragments,
3741
maxPayload,
3842
port: 0
3943
},
@@ -43,6 +47,11 @@ describe('WebSocketServer', () => {
4347
);
4448

4549
wss.on('connection', (ws) => {
50+
assert.strictEqual(
51+
ws._receiver._maxBufferedChunks,
52+
maxBufferedChunks
53+
);
54+
assert.strictEqual(ws._receiver._maxFragments, maxFragments);
4655
assert.strictEqual(ws._receiver._maxPayload, maxPayload);
4756
assert.strictEqual(
4857
ws._receiver._extensions['permessage-deflate']._maxPayload,

‎test/websocket.test.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ describe('WebSocket', () => {
6565
assert.strictEqual(count, 3);
6666
});
6767

68-
it('accepts the `maxPayload` option', (done) => {
68+
it('accepts the receiver limit options', (done) => {
69+
const maxBufferedChunks = 1024;
70+
const maxFragments = 512;
6971
const maxPayload = 20480;
7072
const wss = new WebSocket.Server(
7173
{
@@ -75,10 +77,17 @@ describe('WebSocket', () => {
7577
() => {
7678
const ws = new WebSocket(`ws://localhost:${wss.address().port}`, {
7779
perMessageDeflate: true,
80+
maxBufferedChunks,
81+
maxFragments,
7882
maxPayload
7983
});
8084

8185
ws.on('open', () => {
86+
assert.strictEqual(
87+
ws._receiver._maxBufferedChunks,
88+
maxBufferedChunks
89+
);
90+
assert.strictEqual(ws._receiver._maxFragments, maxFragments);
8291
assert.strictEqual(ws._receiver._maxPayload, maxPayload);
8392
assert.strictEqual(
8493
ws._receiver._extensions['permessage-deflate']._maxPayload,

0 commit comments

Comments
 (0)