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

Check origin header for websocket connection #1626

Merged
merged 3 commits into from Mar 22, 2019
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
32 changes: 27 additions & 5 deletions lib/Server.js
Expand Up @@ -24,6 +24,22 @@ const webpackDevMiddleware = require('webpack-dev-middleware');
const OptionsValidationError = require('./OptionsValidationError');
const optionsSchema = require('./optionsSchema.json');

// Workaround for sockjs@~0.3.19
// sockjs will remove Origin header, however Origin header is required for checking host.
// See https://github.com/webpack/webpack-dev-server/issues/1604 for more information
{
// eslint-disable-next-line global-require
const SockjsSession = require('sockjs/lib/transport').Session;
const decorateConnection = SockjsSession.prototype.decorateConnection;
SockjsSession.prototype.decorateConnection = function(req) {
decorateConnection.call(this, req);
const connection = this.connection;
if (connection.headers && !('origin' in connection.headers) && 'origin' in req.headers) {
connection.headers.origin = req.headers.origin;
}
};
}

const clientStats = { errorDetails: false };
const log = console.log; // eslint-disable-line no-console

Expand Down Expand Up @@ -510,17 +526,23 @@ Server.prototype.setContentHeaders = function (req, res, next) {
next();
};

Server.prototype.checkHost = function (headers) {
Server.prototype.checkHost = function (headers, headerToCheck) {
// allow user to opt-out this security check, at own risk
if (this.disableHostCheck) return true;

if (!headerToCheck) headerToCheck = 'host';
// get the Host header and extract hostname
// we don't care about port not matching
const hostHeader = headers.host;
const hostHeader = headers[headerToCheck];
if (!hostHeader) return false;

// use the node url-parser to retrieve the hostname from the host-header.
const hostname = url.parse(`//${hostHeader}`, false, true).hostname;
const hostname = url.parse(
// if hostHeader doesn't have scheme, add // for parsing.
/^(.+:)?\/\//.test(hostHeader) ? hostHeader : `//${hostHeader}`,
false,
true,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Timsly, would you mind to delete this comma with another PR? It causes issues for node.js <= 7.x
See issue #1607 and the PR that fixed it #1609

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ifnoword Done - #1736

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Timsly I appreciate it. Thank you.

).hostname;

// always allow requests with explicit IPv4 or IPv6-address.
// A note on IPv6 addresses: hostHeader will always contain the brackets denoting
Expand Down Expand Up @@ -581,8 +603,8 @@ Server.prototype.listen = function (port, hostname, fn) {

sockServer.on('connection', (conn) => {
if (!conn) return;
if (!this.checkHost(conn.headers)) {
this.sockWrite([conn], 'error', 'Invalid Host header');
if (!this.checkHost(conn.headers) || !this.checkHost(conn.headers, 'origin')) {
this.sockWrite([conn], 'error', 'Invalid Host/Origin header');
conn.close();
return;
}
Expand Down
13 changes: 13 additions & 0 deletions test/Validation.test.js
Expand Up @@ -144,6 +144,19 @@ describe('Validation', () => {
}
});

it('should allow urls with scheme for checking origin', () => {
const options = {
public: 'test.host:80'
};
const headers = {
origin: 'https://test.host'
};
const server = new Server(compiler, options);
if (!server.checkHost(headers, 'origin')) {
throw new Error("Validation didn't fail");
}
});

describe('allowedHosts', () => {
it('should allow hosts in allowedHosts', () => {
const testHosts = [
Expand Down