Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close the connection if the MASK bit is incorrectly set #1681

Merged
merged 1 commit into from Jan 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions bench/parser.benchmark.js
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
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
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
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
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
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.prependListener('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