Skip to content

Commit e14c458

Browse files
Nadav0077lpinca
authored andcommitted
[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 d962d70 commit e14c458

7 files changed

Lines changed: 209 additions & 7 deletions

File tree

doc/ws.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
- [WS_ERR_UNEXPECTED_MASK](#ws_err_unexpected_mask)
5555
- [WS_ERR_UNEXPECTED_RSV_1](#ws_err_unexpected_rsv_1)
5656
- [WS_ERR_UNEXPECTED_RSV_2_3](#ws_err_unexpected_rsv_2_3)
57+
- [WS_ERR_TOO_MANY_BUFFERED_PARTS](#ws_err_too_many_buffered_parts)
5758
- [WS_ERR_UNSUPPORTED_DATA_PAYLOAD_LENGTH](#ws_err_unsupported_data_payload_length)
5859
- [WS_ERR_UNSUPPORTED_MESSAGE_LENGTH](#ws_err_unsupported_message_length)
5960

@@ -77,6 +78,10 @@ This class represents a WebSocket server. It extends the `EventEmitter`.
7778
- `noServer` {Boolean} Enable no server mode.
7879
- `clientTracking` {Boolean} Specifies whether or not to track clients.
7980
- `perMessageDeflate` {Boolean|Object} Enable/disable permessage-deflate.
81+
- `maxBufferedChunks` {Number} The maximum number of buffered data chunks.
82+
Defaults to 1048576. Set to 0 to disable the limit.
83+
- `maxFragments` {Number} The maximum number of fragments in a message.
84+
Defaults to 131072. Set to 0 to disable the limit.
8085
- `maxPayload` {Number} The maximum allowed message size in bytes.
8186
- `callback` {Function}
8287

@@ -267,6 +272,10 @@ This class represents a WebSocket. It extends the `EventEmitter`.
267272
- `protocolVersion` {Number} Value of the `Sec-WebSocket-Version` header.
268273
- `origin` {String} Value of the `Origin` or `Sec-WebSocket-Origin` header
269274
depending on the `protocolVersion`.
275+
- `maxBufferedChunks` {Number} The maximum number of buffered data chunks.
276+
Defaults to 1048576. Set to 0 to disable the limit.
277+
- `maxFragments` {Number} The maximum number of fragments in a message.
278+
Defaults to 131072. Set to 0 to disable the limit.
270279
- `maxPayload` {Number} The maximum allowed message size in bytes.
271280
- Any other option allowed in [http.request()][] or [https.request()][].
272281
Options given do not have any effect if parsed from the URL given with the
@@ -547,6 +556,11 @@ A WebSocket frame was received with the RSV1 bit set unexpectedly.
547556

548557
A WebSocket frame was received with the RSV2 or RSV3 bit set unexpectedly.
549558

559+
### WS_ERR_TOO_MANY_BUFFERED_PARTS
560+
561+
The configured maximum number of buffered data chunks or message fragments was
562+
exceeded.
563+
550564
### WS_ERR_UNSUPPORTED_DATA_PAYLOAD_LENGTH
551565

552566
A data frame was received with a length longer than the max supported length

lib/receiver.js

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,27 @@ class Receiver extends Writable {
3333
* @param {Boolean} [isServer=false] Specifies whether to operate in client or
3434
* server mode
3535
* @param {Number} [maxPayload=0] The maximum allowed message length
36+
* @param {Number} [maxBufferedChunks=0] The maximum number of
37+
* buffered data chunks
38+
* @param {Number} [maxFragments=0] The maximum number of message
39+
* fragments
3640
*/
37-
constructor(binaryType, extensions, isServer, maxPayload) {
41+
constructor(
42+
binaryType,
43+
extensions,
44+
isServer,
45+
maxPayload,
46+
maxBufferedChunks,
47+
maxFragments
48+
) {
3849
super();
3950

4051
this._binaryType = binaryType || BINARY_TYPES[0];
4152
this[kWebSocket] = undefined;
4253
this._extensions = extensions || {};
4354
this._isServer = !!isServer;
55+
this._maxBufferedChunks = maxBufferedChunks | 0;
56+
this._maxFragments = maxFragments | 0;
4457
this._maxPayload = maxPayload | 0;
4558

4659
this._bufferedBytes = 0;
@@ -73,6 +86,21 @@ class Receiver extends Writable {
7386
_write(chunk, encoding, cb) {
7487
if (this._opcode === 0x08 && this._state == GET_INFO) return cb();
7588

89+
if (
90+
this._maxBufferedChunks > 0 &&
91+
this._buffers.length >= this._maxBufferedChunks
92+
) {
93+
return cb(
94+
error(
95+
RangeError,
96+
'Too many buffered chunks',
97+
false,
98+
1008,
99+
'WS_ERR_TOO_MANY_BUFFERED_PARTS'
100+
)
101+
);
102+
}
103+
76104
this._bufferedBytes += chunk.length;
77105
this._buffers.push(chunk);
78106
this.startLoop(cb);
@@ -424,6 +452,20 @@ class Receiver extends Writable {
424452
}
425453

426454
if (data.length) {
455+
if (
456+
this._maxFragments > 0 &&
457+
this._fragments.length >= this._maxFragments
458+
) {
459+
this._loop = false;
460+
return error(
461+
RangeError,
462+
'Too many message fragments',
463+
false,
464+
1008,
465+
'WS_ERR_TOO_MANY_BUFFERED_PARTS'
466+
);
467+
}
468+
427469
//
428470
// This message is not compressed so its lenght is the sum of the payload
429471
// length of all fragments.
@@ -462,6 +504,21 @@ class Receiver extends Writable {
462504
);
463505
}
464506

507+
if (
508+
this._maxFragments > 0 &&
509+
this._fragments.length >= this._maxFragments
510+
) {
511+
return cb(
512+
error(
513+
RangeError,
514+
'Too many message fragments',
515+
false,
516+
1008,
517+
'WS_ERR_TOO_MANY_BUFFERED_PARTS'
518+
)
519+
);
520+
}
521+
465522
this._fragments.push(buf);
466523
}
467524

lib/websocket-server.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ class WebSocketServer extends EventEmitter {
3636
* track clients
3737
* @param {Function} [options.handleProtocols] A hook to handle protocols
3838
* @param {String} [options.host] The hostname where to bind the server
39+
* @param {Number} [options.maxBufferedChunks=1048576] The maximum number of
40+
* buffered data chunks
41+
* @param {Number} [options.maxFragments=131072] The maximum number of message
42+
* fragments
3943
* @param {Number} [options.maxPayload=104857600] The maximum allowed message
4044
* size
4145
* @param {Boolean} [options.noServer=false] Enable no server mode
@@ -52,6 +56,8 @@ class WebSocketServer extends EventEmitter {
5256
super();
5357

5458
options = {
59+
maxBufferedChunks: 1024 * 1024,
60+
maxFragments: 128 * 1024,
5561
maxPayload: 100 * 1024 * 1024,
5662
perMessageDeflate: false,
5763
handleProtocols: null,
@@ -350,7 +356,13 @@ class WebSocketServer extends EventEmitter {
350356
socket.write(headers.concat('\r\n').join('\r\n'));
351357
socket.removeListener('error', socketOnError);
352358

353-
ws.setSocket(socket, head, this.options.maxPayload);
359+
ws.setSocket(
360+
socket,
361+
head,
362+
this.options.maxPayload,
363+
this.options.maxBufferedChunks,
364+
this.options.maxFragments
365+
);
354366

355367
if (this.clients) {
356368
this.clients.add(ws);

lib/websocket.js

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,14 +187,20 @@ class WebSocket extends EventEmitter {
187187
* server and client
188188
* @param {Buffer} head The first packet of the upgraded stream
189189
* @param {Number} [maxPayload=0] The maximum allowed message size
190+
* @param {Number} [maxBufferedChunks=0] The maximum number of
191+
* buffered data chunks
192+
* @param {Number} [maxFragments=0] The maximum number of message
193+
* fragments
190194
* @private
191195
*/
192-
setSocket(socket, head, maxPayload) {
196+
setSocket(socket, head, maxPayload, maxBufferedChunks, maxFragments) {
193197
const receiver = new Receiver(
194198
this.binaryType,
195199
this._extensions,
196200
this._isServer,
197-
maxPayload
201+
maxPayload,
202+
maxBufferedChunks,
203+
maxFragments
198204
);
199205

200206
this._sender = new Sender(socket, this._extensions);
@@ -571,6 +577,10 @@ module.exports = WebSocket;
571577
* `Sec-WebSocket-Version` header
572578
* @param {String} [options.origin] Value of the `Origin` or
573579
* `Sec-WebSocket-Origin` header
580+
* @param {Number} [options.maxBufferedChunks=1048576] The maximum number of
581+
* buffered data chunks
582+
* @param {Number} [options.maxFragments=131072] The maximum number of message
583+
* fragments
574584
* @param {Number} [options.maxPayload=104857600] The maximum allowed message
575585
* size
576586
* @param {Boolean} [options.followRedirects=false] Whether or not to follow
@@ -582,6 +592,8 @@ module.exports = WebSocket;
582592
function initAsClient(websocket, address, protocols, options) {
583593
const opts = {
584594
protocolVersion: protocolVersions[1],
595+
maxBufferedChunks: 1024 * 1024,
596+
maxFragments: 128 * 1024,
585597
maxPayload: 100 * 1024 * 1024,
586598
perMessageDeflate: true,
587599
followRedirects: false,
@@ -881,7 +893,13 @@ function initAsClient(websocket, address, protocols, options) {
881893
}
882894
}
883895

884-
websocket.setSocket(socket, head, opts.maxPayload);
896+
websocket.setSocket(
897+
socket,
898+
head,
899+
opts.maxPayload,
900+
opts.maxBufferedChunks,
901+
opts.maxFragments
902+
);
885903
});
886904
}
887905

test/receiver.test.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,89 @@ describe('Receiver', () => {
955955
});
956956
});
957957

958+
it('emits an error if there are too many message fragments (1/2)', (done) => {
959+
const receiver = new Receiver(undefined, {}, false, 0, 0, 2);
960+
961+
receiver.on('error', (err) => {
962+
assert.ok(err instanceof RangeError);
963+
assert.strictEqual(err.code, 'WS_ERR_TOO_MANY_BUFFERED_PARTS');
964+
assert.strictEqual(err.message, 'Too many message fragments');
965+
assert.strictEqual(err[kStatusCode], 1008);
966+
done();
967+
});
968+
969+
receiver.write(
970+
Buffer.from([
971+
0x02,
972+
0x01,
973+
0x61, // First non-final binary fragment.
974+
0x00,
975+
0x01,
976+
0x62, // Continuation fragment.
977+
0x00,
978+
0x01,
979+
0x63 // Continuation fragment that exceeds the limit.
980+
])
981+
);
982+
});
983+
984+
it('emits an error if there are too many message fragments (2/2)', (done) => {
985+
const perMessageDeflate = new PerMessageDeflate();
986+
perMessageDeflate.accept([{}]);
987+
988+
const receiver = new Receiver(
989+
undefined,
990+
{
991+
'permessage-deflate': perMessageDeflate
992+
},
993+
false,
994+
0,
995+
0,
996+
1
997+
);
998+
const fragment1 = Buffer.from('foo');
999+
const fragment2 = Buffer.from('bar');
1000+
1001+
receiver.on('error', (err) => {
1002+
assert.ok(err instanceof RangeError);
1003+
assert.strictEqual(err.code, 'WS_ERR_TOO_MANY_BUFFERED_PARTS');
1004+
assert.strictEqual(err.message, 'Too many message fragments');
1005+
assert.strictEqual(err[kStatusCode], 1008);
1006+
done();
1007+
});
1008+
1009+
perMessageDeflate.compress(fragment1, false, (err, data) => {
1010+
if (err) return done(err);
1011+
1012+
receiver.write(Buffer.from([0x41, data.length]));
1013+
receiver.write(data);
1014+
1015+
perMessageDeflate.compress(fragment2, true, (err, data) => {
1016+
if (err) return done(err);
1017+
1018+
receiver.write(Buffer.from([0x80, data.length]));
1019+
receiver.write(data);
1020+
});
1021+
});
1022+
});
1023+
1024+
it('emits an error if there are too many buffered chunks', (done) => {
1025+
const receiver = new Receiver(undefined, {}, false, 0, 2);
1026+
1027+
receiver.on('error', (err) => {
1028+
assert.ok(err instanceof RangeError);
1029+
assert.strictEqual(err.code, 'WS_ERR_TOO_MANY_BUFFERED_PARTS');
1030+
assert.strictEqual(err.message, 'Too many buffered chunks');
1031+
assert.strictEqual(err[kStatusCode], 1008);
1032+
done();
1033+
});
1034+
1035+
receiver.write(Buffer.from([0x82, 0x05]));
1036+
receiver.write(Buffer.from([0x61]));
1037+
receiver.write(Buffer.from([0x62]));
1038+
receiver.write(Buffer.from([0x63]));
1039+
});
1040+
9581041
it("honors the 'nodebuffer' binary type", (done) => {
9591042
const receiver = new Receiver();
9601043
const frags = [

test/websocket-server.test.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,15 @@ describe('WebSocketServer', () => {
6565
});
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
{
7274
perMessageDeflate: true,
75+
maxBufferedChunks,
76+
maxFragments,
7377
maxPayload,
7478
port: 0
7579
},
@@ -79,6 +83,11 @@ describe('WebSocketServer', () => {
7983
);
8084

8185
wss.on('connection', (ws) => {
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,

test/websocket.test.js

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

60-
it('accepts the `maxPayload` option', (done) => {
60+
it('accepts the receiver limit options', (done) => {
61+
const maxBufferedChunks = 1024;
62+
const maxFragments = 512;
6163
const maxPayload = 20480;
6264
const wss = new WebSocket.Server(
6365
{
@@ -67,10 +69,17 @@ describe('WebSocket', () => {
6769
() => {
6870
const ws = new WebSocket(`ws://localhost:${wss.address().port}`, {
6971
perMessageDeflate: true,
72+
maxBufferedChunks,
73+
maxFragments,
7074
maxPayload
7175
});
7276

7377
ws.on('open', () => {
78+
assert.strictEqual(
79+
ws._receiver._maxBufferedChunks,
80+
maxBufferedChunks
81+
);
82+
assert.strictEqual(ws._receiver._maxFragments, maxFragments);
7483
assert.strictEqual(ws._receiver._maxPayload, maxPayload);
7584
assert.strictEqual(
7685
ws._receiver._extensions['permessage-deflate']._maxPayload,

0 commit comments

Comments
 (0)