Skip to content

Commit

Permalink
[fix] Abort the handshake if the Sec-WebSocket-Key header is invalid
Browse files Browse the repository at this point in the history
  • Loading branch information
@lpinca
lpinca committed Feb 22, 2019
1 parent 1d93fb2 commit 160af45
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 23 deletions.
24 changes: 16 additions & 8 deletions lib/websocket-server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ const http = require('http');

const PerMessageDeflate = require('./permessage-deflate');
const extension = require('./extension');
const constants = require('./constants');
const WebSocket = require('./websocket');
const { GUID } = require('./constants');

const keyRegex = /^[+/0-9A-Za-z]{22}==$/;

/**
* Class representing a WebSocket server.
Expand Down Expand Up @@ -176,13 +178,18 @@ class WebSocketServer extends EventEmitter {
handleUpgrade(req, socket, head, cb) {
socket.on('error', socketOnError);

const key =
req.headers['sec-websocket-key'] !== undefined
? req.headers['sec-websocket-key'].trim()
: false;
const version = +req.headers['sec-websocket-version'];
const extensions = {};

if (
req.method !== 'GET' ||
req.headers.upgrade.toLowerCase() !== 'websocket' ||
!req.headers['sec-websocket-key'] ||
!key ||
!keyRegex.test(key) ||
(version !== 8 && version !== 13) ||
!this.shouldHandle(req)
) {
Expand Down Expand Up @@ -225,43 +232,44 @@ class WebSocketServer extends EventEmitter {
return abortHandshake(socket, code || 401, message, headers);
}

this.completeUpgrade(extensions, req, socket, head, cb);
this.completeUpgrade(key, extensions, req, socket, head, cb);
});
return;
}

if (!this.options.verifyClient(info)) return abortHandshake(socket, 401);
}

this.completeUpgrade(extensions, req, socket, head, cb);
this.completeUpgrade(key, extensions, req, socket, head, cb);
}

/**
* Upgrade the connection to WebSocket.
*
* @param {String} key The value of the `Sec-WebSocket-Key` header
* @param {Object} extensions The accepted extensions
* @param {http.IncomingMessage} req The request object
* @param {net.Socket} socket The network socket between the server and client
* @param {Buffer} head The first packet of the upgraded stream
* @param {Function} cb Callback
* @private
*/
completeUpgrade(extensions, req, socket, head, cb) {
completeUpgrade(key, extensions, req, socket, head, cb) {
//
// Destroy the socket if the client has already sent a FIN packet.
//
if (!socket.readable || !socket.writable) return socket.destroy();

const key = crypto
const digest = crypto
.createHash('sha1')
.update(req.headers['sec-websocket-key'] + constants.GUID, 'binary')
.update(key + GUID)
.digest('base64');

const headers = [
'HTTP/1.1 101 Switching Protocols',
'Upgrade: websocket',
'Connection: Upgrade',
`Sec-WebSocket-Accept: ${key}`
`Sec-WebSocket-Accept: ${digest}`
];

const ws = new WebSocket(null);
Expand Down
28 changes: 17 additions & 11 deletions lib/websocket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,18 @@ const url = require('url');
const PerMessageDeflate = require('./permessage-deflate');
const EventTarget = require('./event-target');
const extension = require('./extension');
const constants = require('./constants');
const Receiver = require('./receiver');
const Sender = require('./sender');
const {
BINARY_TYPES,
EMPTY_BUFFER,
GUID,
kStatusCode,
kWebSocket,
NOOP
} = require('./constants');

const readyStates = ['CONNECTING', 'OPEN', 'CLOSING', 'CLOSED'];
const kWebSocket = constants.kWebSocket;
const protocolVersions = [8, 13];
const closeTimeout = 30 * 1000; // Allow 30 seconds to terminate the connection cleanly.

Expand All @@ -39,7 +45,7 @@ class WebSocket extends EventEmitter {
this.readyState = WebSocket.CONNECTING;
this.protocol = '';

this._binaryType = constants.BINARY_TYPES[0];
this._binaryType = BINARY_TYPES[0];
this._closeFrameReceived = false;
this._closeFrameSent = false;
this._closeMessage = '';
Expand Down Expand Up @@ -87,7 +93,7 @@ class WebSocket extends EventEmitter {
}

set binaryType(type) {
if (!constants.BINARY_TYPES.includes(type)) return;
if (!BINARY_TYPES.includes(type)) return;

this._binaryType = type;

Expand Down Expand Up @@ -265,7 +271,7 @@ class WebSocket extends EventEmitter {

if (typeof data === 'number') data = data.toString();
if (mask === undefined) mask = !this._isServer;
this._sender.ping(data || constants.EMPTY_BUFFER, mask, cb);
this._sender.ping(data || EMPTY_BUFFER, mask, cb);
}

/**
Expand Down Expand Up @@ -297,7 +303,7 @@ class WebSocket extends EventEmitter {

if (typeof data === 'number') data = data.toString();
if (mask === undefined) mask = !this._isServer;
this._sender.pong(data || constants.EMPTY_BUFFER, mask, cb);
this._sender.pong(data || EMPTY_BUFFER, mask, cb);
}

/**
Expand Down Expand Up @@ -344,7 +350,7 @@ class WebSocket extends EventEmitter {
opts.compress = false;
}

this._sender.send(data || constants.EMPTY_BUFFER, opts, cb);
this._sender.send(data || EMPTY_BUFFER, opts, cb);
}

/**
Expand Down Expand Up @@ -574,7 +580,7 @@ function initAsClient(address, protocols, options) {

const digest = crypto
.createHash('sha1')
.update(key + constants.GUID, 'binary')
.update(key + GUID)
.digest('base64');

if (res.headers['sec-websocket-accept'] !== digest) {
Expand Down Expand Up @@ -720,7 +726,7 @@ function receiverOnError(err) {
websocket._socket.removeListener('data', socketOnData);

websocket.readyState = WebSocket.CLOSING;
websocket._closeCode = err[constants.kStatusCode];
websocket._closeCode = err[kStatusCode];
websocket.emit('error', err);
websocket._socket.destroy();
}
Expand Down Expand Up @@ -753,7 +759,7 @@ function receiverOnMessage(data) {
function receiverOnPing(data) {
const websocket = this[kWebSocket];

websocket.pong(data, !websocket._isServer, constants.NOOP);
websocket.pong(data, !websocket._isServer, NOOP);
websocket.emit('ping', data);
}

Expand Down Expand Up @@ -843,7 +849,7 @@ function socketOnError() {
const websocket = this[kWebSocket];

this.removeListener('error', socketOnError);
this.on('error', constants.NOOP);
this.on('error', NOOP);

if (websocket) {
websocket.readyState = WebSocket.CLOSING;
Expand Down
24 changes: 23 additions & 1 deletion test/websocket-server.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ describe('WebSocketServer', function() {
});

describe('Connection establishing', function() {
it('fails if the Sec-WebSocket-Key header is invalid', function(done) {
it('fails if the Sec-WebSocket-Key header is invalid (1/2)', function(done) {
const wss = new WebSocket.Server({ port: 0 }, () => {
const req = http.get({
port: wss.address().port,
Expand All @@ -404,6 +404,28 @@ describe('WebSocketServer', function() {
});
});

it('fails if the Sec-WebSocket-Key header is invalid (2/2)', function(done) {
const wss = new WebSocket.Server({ port: 0 }, () => {
const req = http.get({
port: wss.address().port,
headers: {
Connection: 'Upgrade',
Upgrade: 'websocket',
'Sec-WebSocket-Key': 'P5l8BJcZwRc='
}
});

req.on('response', (res) => {
assert.strictEqual(res.statusCode, 400);
wss.close(done);
});
});

wss.on('connection', () => {
done(new Error("Unexpected 'connection' event"));
});
});

it('fails is the Sec-WebSocket-Version header is invalid (1/2)', function(done) {
const wss = new WebSocket.Server({ port: 0 }, () => {
const req = http.get({
Expand Down
6 changes: 3 additions & 3 deletions test/websocket.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const http = require('http');
const url = require('url');
const fs = require('fs');

const constants = require('../lib/constants');
const WebSocket = require('..');
const { GUID } = require('../lib/constants');

class CustomAgent extends http.Agent {
addRequest() {}
Expand Down Expand Up @@ -463,7 +463,7 @@ describe('WebSocket', function() {
server.once('upgrade', (req, socket) => {
const key = crypto
.createHash('sha1')
.update(req.headers['sec-websocket-key'] + constants.GUID, 'binary')
.update(req.headers['sec-websocket-key'] + GUID)
.digest('base64');

socket.end(
Expand Down Expand Up @@ -581,7 +581,7 @@ describe('WebSocket', function() {
server.once('upgrade', (req, socket) => {
const key = crypto
.createHash('sha1')
.update(req.headers['sec-websocket-key'] + constants.GUID, 'binary')
.update(req.headers['sec-websocket-key'] + GUID)
.digest('base64');

socket.end(
Expand Down

0 comments on commit 160af45

Please sign in to comment.