Skip to content

Commit

Permalink
check origin header for websocket connection
Browse files Browse the repository at this point in the history
  • Loading branch information
sokra committed Jul 24, 2018
1 parent e1bd264 commit f18e5ad
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions lib/Server.js
Expand Up @@ -513,13 +513,15 @@ 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.
Expand Down Expand Up @@ -589,6 +591,11 @@ Server.prototype.listen = function (port, hostname, fn) {
conn.close();
return;
}
if (!this.checkHost(conn.headers, "origin")) {
this.sockWrite([conn], 'error', 'Invalid Origin header');
conn.close();
return;
}
this.sockets.push(conn);

conn.on('close', () => {
Expand Down

5 comments on commit f18e5ad

@hackel
Copy link

@hackel hackel commented on f18e5ad Nov 9, 2018

Choose a reason for hiding this comment

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

Any chance this security fix could be backported to 2.x?
Just noticed this. https://nodesecurity.io/advisories/725

@alexander-akait
Copy link
Member

Choose a reason for hiding this comment

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

No, please update to 3 version, 2 is deprecated

@aeegvk
Copy link

@aeegvk aeegvk commented on f18e5ad Jan 2, 2019

Choose a reason for hiding this comment

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

Updated to suggested version 3.1.11 and latest version 3.1.14 but still getting a vulnerability report. How come?

@oles
Copy link
Contributor

@oles oles commented on f18e5ad Jan 2, 2019

Choose a reason for hiding this comment

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

Experiencing the same as @aeegvk.

Seems like the error is in https://www.npmjs.com/advisories/725 though.

@charlesfaustin
Copy link

Choose a reason for hiding this comment

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

Updated to suggested version 3.1.11 and latest version 3.1.14 but still getting a vulnerability report. How come?

there appears to be a typo in the npm vulnerability database
https://npm.community/t/npm-audit-sweems-to-get-semver-wrong/4352/4

Please sign in to comment.