Skip to content

Commit

Permalink
[fix] Close the connection if the MASK bit is incorrectly set
Browse files Browse the repository at this point in the history 
Follow the specification and close the connection when a non masked
frame is received on the server or a masked frame is received on the
client.

Refs: https://tools.ietf.org/html/rfc6455#section-5.1

Closes #1679
  • Loading branch information
@lpinca
lpinca committed Jan 19, 2020
1 parent 748a844 commit cbab6a7
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 40 deletions.
4 changes: 2 additions & 2 deletions bench/parser.benchmark.js
View file
Expand Up @@ -29,14 +29,14 @@ const pingFrame1 = Buffer.concat(
);

const textFrame = Buffer.from('819461616161' + '61'.repeat(20), 'hex');
const pingFrame2 = Buffer.from('8900', 'hex');
const pingFrame2 = Buffer.from('8980146e915a', 'hex');
const binaryFrame1 = createBinaryFrame(125);
const binaryFrame2 = createBinaryFrame(65535);
const binaryFrame3 = createBinaryFrame(200 * 1024);
const binaryFrame4 = createBinaryFrame(1024 * 1024);

const suite = new benchmark.Suite();
const receiver = new Receiver();
const receiver = new Receiver('nodebuffer', {}, true);

suite.add('ping frame (5 bytes payload)', {
defer: true,
Expand Down
15 changes: 14 additions & 1 deletion lib/receiver.js
View file
Expand Up @@ -30,14 +30,17 @@ class Receiver extends Writable {
*
* @param {String} binaryType The type for binary data
* @param {Object} extensions An object containing the negotiated extensions
* @param {Boolean} isServer Specifies whether to operate in client or server
* mode
* @param {Number} maxPayload The maximum allowed message length
*/
constructor(binaryType, extensions, maxPayload) {
constructor(binaryType, extensions, isServer, maxPayload) {
super();

this._binaryType = binaryType || BINARY_TYPES[0];
this[kWebSocket] = undefined;
this._extensions = extensions || {};
this._isServer = !!isServer;
this._maxPayload = maxPayload | 0;

this._bufferedBytes = 0;
Expand Down Expand Up @@ -225,6 +228,16 @@ class Receiver extends Writable {
if (!this._fin && !this._fragmented) this._fragmented = this._opcode;
this._masked = (buf[1] & 0x80) === 0x80;

if (this._isServer) {
if (!this._masked) {
this._loop = false;
return error(RangeError, 'MASK must be set', true, 1002);
}
} else if (this._masked) {
this._loop = false;
return error(RangeError, 'MASK must be clear', true, 1002);
}

if (this._payloadLength === 126) this._state = GET_PAYLOAD_LENGTH_16;
else if (this._payloadLength === 127) this._state = GET_PAYLOAD_LENGTH_64;
else return this.haveLength();
Expand Down
1 change: 1 addition & 0 deletions lib/websocket.js
View file
Expand Up @@ -141,6 +141,7 @@ class WebSocket extends EventEmitter {
const receiver = new Receiver(
this._binaryType,
this._extensions,
this._isServer,
maxPayload
);

Expand Down
64 changes: 50 additions & 14 deletions test/receiver.test.js
View file
Expand Up @@ -48,7 +48,7 @@ describe('Receiver', () => {
});

it('parses a masked text message', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);

receiver.on('message', (data) => {
assert.strictEqual(data, '5:::{"name":"echo"}');
Expand All @@ -61,7 +61,7 @@ describe('Receiver', () => {
});

it('parses a masked text message longer than 125 B', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = 'A'.repeat(200);

const list = Sender.frame(Buffer.from(msg), {
Expand All @@ -84,7 +84,7 @@ describe('Receiver', () => {
});

it('parses a really long masked text message', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = 'A'.repeat(64 * 1024);

const list = Sender.frame(Buffer.from(msg), {
Expand All @@ -106,7 +106,7 @@ describe('Receiver', () => {
});

it('parses a 300 B fragmented masked text message', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = 'A'.repeat(300);

const fragment1 = msg.substr(0, 150);
Expand Down Expand Up @@ -139,7 +139,7 @@ describe('Receiver', () => {
});

it('parses a ping message', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = 'Hello';

const list = Sender.frame(Buffer.from(msg), {
Expand Down Expand Up @@ -172,7 +172,7 @@ describe('Receiver', () => {
});

it('parses a 300 B fragmented masked text message with a ping in the middle (1/2)', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = 'A'.repeat(300);
const pingMessage = 'Hello';

Expand Down Expand Up @@ -221,7 +221,7 @@ describe('Receiver', () => {
});

it('parses a 300 B fragmented masked text message with a ping in the middle (2/2)', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = 'A'.repeat(300);
const pingMessage = 'Hello';

Expand Down Expand Up @@ -280,7 +280,7 @@ describe('Receiver', () => {
});

it('parses a 100 B masked binary message', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = crypto.randomBytes(100);

const list = Sender.frame(msg, {
Expand All @@ -302,7 +302,7 @@ describe('Receiver', () => {
});

it('parses a 256 B masked binary message', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = crypto.randomBytes(256);

const list = Sender.frame(msg, {
Expand All @@ -324,7 +324,7 @@ describe('Receiver', () => {
});

it('parses a 200 KiB masked binary message', (done) => {
const receiver = new Receiver();
const receiver = new Receiver(undefined, {}, true);
const msg = crypto.randomBytes(200 * 1024);

const list = Sender.frame(msg, {
Expand Down Expand Up @@ -439,7 +439,7 @@ describe('Receiver', () => {
});

it('resets `totalPayloadLength` only on final frame (unfragmented)', (done) => {
const receiver = new Receiver(undefined, {}, 10);
const receiver = new Receiver(undefined, {}, false, 10);

receiver.on('message', (data) => {
assert.strictEqual(receiver._totalPayloadLength, 0);
Expand All @@ -452,7 +452,7 @@ describe('Receiver', () => {
});

it('resets `totalPayloadLength` only on final frame (fragmented)', (done) => {
const receiver = new Receiver(undefined, {}, 10);
const receiver = new Receiver(undefined, {}, false, 10);

receiver.on('message', (data) => {
assert.strictEqual(receiver._totalPayloadLength, 0);
Expand All @@ -467,7 +467,7 @@ describe('Receiver', () => {
});

it('resets `totalPayloadLength` only on final frame (fragmented + ping)', (done) => {
const receiver = new Receiver(undefined, {}, 10);
const receiver = new Receiver(undefined, {}, false, 10);
let data;

receiver.on('ping', (buf) => {
Expand Down Expand Up @@ -680,6 +680,40 @@ describe('Receiver', () => {
receiver.write(Buffer.from([0x09, 0x00]));
});

it('emits an error if a frame has the MASK bit off (server mode)', (done) => {
const receiver = new Receiver(undefined, {}, true);

receiver.on('error', (err) => {
assert.ok(err instanceof RangeError);
assert.strictEqual(
err.message,
'Invalid WebSocket frame: MASK must be set'
);
assert.strictEqual(err[kStatusCode], 1002);
done();
});

receiver.write(Buffer.from([0x81, 0x02, 0x68, 0x69]));
});

it('emits an error if a frame has the MASK bit on (client mode)', (done) => {
const receiver = new Receiver(undefined, {}, false);

receiver.on('error', (err) => {
assert.ok(err instanceof RangeError);
assert.strictEqual(
err.message,
'Invalid WebSocket frame: MASK must be clear'
);
assert.strictEqual(err[kStatusCode], 1002);
done();
});

receiver.write(
Buffer.from([0x81, 0x82, 0x56, 0x3a, 0xac, 0x80, 0x3e, 0x53])
);
});

it('emits an error if a control frame has a payload bigger than 125 B', (done) => {
const receiver = new Receiver();

Expand Down Expand Up @@ -811,7 +845,7 @@ describe('Receiver', () => {
});

it('emits an error if a frame payload length is bigger than `maxPayload`', (done) => {
const receiver = new Receiver(undefined, {}, 20 * 1024);
const receiver = new Receiver(undefined, {}, true, 20 * 1024);
const msg = crypto.randomBytes(200 * 1024);

const list = Sender.frame(msg, {
Expand Down Expand Up @@ -843,6 +877,7 @@ describe('Receiver', () => {
{
'permessage-deflate': perMessageDeflate
},
false,
25
);
const buf = Buffer.from('A'.repeat(50));
Expand Down Expand Up @@ -871,6 +906,7 @@ describe('Receiver', () => {
{
'permessage-deflate': perMessageDeflate
},
false,
25
);
const buf = Buffer.from('A'.repeat(15));
Expand Down
11 changes: 10 additions & 1 deletion test/websocket-server.test.js
View file
Expand Up @@ -9,6 +9,7 @@ const http = require('http');
const net = require('net');
const fs = require('fs');

const Sender = require('../lib/sender');
const WebSocket = require('..');

describe('WebSocketServer', () => {
Expand Down Expand Up @@ -744,7 +745,15 @@ describe('WebSocketServer', () => {
}
});

req.write(Buffer.from([0x81, 0x05, 0x48, 0x65, 0x6c, 0x6c, 0x6f]));
const list = Sender.frame(Buffer.from('Hello'), {
fin: true,
rsv1: false,
opcode: 0x01,
mask: true,
readOnly: false
});

req.write(Buffer.concat(list));
req.end();
});

Expand Down
43 changes: 21 additions & 22 deletions test/websocket.test.js
View file
Expand Up @@ -1293,38 +1293,37 @@ describe('WebSocket', () => {
});
});

it('can send text data with `mask` option set to `false`', (done) => {
it('honors the `mask` option', (done) => {
const wss = new WebSocket.Server({ port: 0 }, () => {
const ws = new WebSocket(`ws://localhost:${wss.address().port}`);

ws.on('open', () => ws.send('hi', { mask: false }));
});

wss.on('connection', (ws) => {
ws.on('message', (message) => {
assert.strictEqual(message, 'hi');
wss.close(done);
});
});
});

it('can send binary data with `mask` option set to `false`', (done) => {
const array = new Float32Array(5);
const chunks = [];

for (let i = 0; i < array.length; ++i) {
array[i] = i / 2;
}

const wss = new WebSocket.Server({ port: 0 }, () => {
const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
ws._socket.on('data', (chunk) => {
chunks.push(chunk);
});

ws.on('open', () => ws.send(array, { mask: false }));
});
ws.on('error', (err) => {
assert.ok(err instanceof RangeError);
assert.strictEqual(
err.message,
'Invalid WebSocket frame: MASK must be set'
);
assert.ok(
Buffer.concat(chunks)
.slice(0, 2)
.equals(Buffer.from('8102', 'hex'))
);

wss.on('connection', (ws) => {
ws.on('message', (message) => {
assert.ok(message.equals(Buffer.from(array.buffer)));
wss.close(done);
ws.on('close', (code, reason) => {
assert.strictEqual(code, 1002);
assert.strictEqual(reason, '');
wss.close(done);
});
});
});
});
Expand Down

0 comments on commit cbab6a7

Please sign in to comment.