From 657a2e55b75a6126d073a133af4b9031867aff2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Requena=20L=C3=B3pez?= Date: Tue, 8 Jan 2019 21:00:54 +0100 Subject: [PATCH 1/3] disallow automatic validation of ALL IP's. fixes #1618 for both Host checking and Origin checking --- lib/Server.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index d76af59010..6542337c8d 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -647,6 +647,10 @@ Server.prototype.setContentHeaders = function (req, res, next) { }; Server.prototype.checkHost = function (headers, headerToCheck) { + /* This routine is also used to check the Origin header, whenever + * headerToCheck says so + */ + // allow user to opt-out this security check, at own risk if (this.disableHostCheck) { return true; @@ -668,15 +672,6 @@ Server.prototype.checkHost = function (headers, headerToCheck) { false, true ).hostname; - // always allow requests with explicit IPv4 or IPv6-address. - // A note on IPv6 addresses: - // hostHeader will always contain the brackets denoting - // an IPv6-address in URLs, - // these are removed from the hostname in url.parse(), - // so we have the pure IPv6-address in hostname. - if (ip.isV4Format(hostname) || ip.isV6Format(hostname)) { - return true; - } // always allow localhost host, for convience if (hostname === 'localhost') { return true; From 0f22eaa2b14cd477e3f1c26069627ffa38029092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Requena=20L=C3=B3pez?= Date: Tue, 8 Jan 2019 22:12:02 +0100 Subject: [PATCH 2/3] adapt tests fix #1618 --- lib/Server.js | 3 ++- test/Validation.test.js | 37 +++++++++++++++++-------------------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index 6542337c8d..7fa373d0fe 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -672,8 +672,9 @@ Server.prototype.checkHost = function (headers, headerToCheck) { false, true ).hostname; + // always allow localhost host, for convience - if (hostname === 'localhost') { + if (hostname === 'localhost' || hostname === '127.0.0.1') { return true; } // allow if hostname is in allowedHosts diff --git a/test/Validation.test.js b/test/Validation.test.js index 8dc0f98cb6..877579bcb0 100644 --- a/test/Validation.test.js +++ b/test/Validation.test.js @@ -132,10 +132,17 @@ describe('Validation', () => { } }); - it('should allow access for every requests using an IP', () => { - const options = {}; + it('should not allow access for IPs or hostnames that are not in options.public or allowedHosts and viceversa', () => { + const options = { + public: 'pointerpointer.com', + allowedHosts: ['realdevdomain.dev'] + }; const tests = [ + 'realdevdomain.dev', + 'test.hostname:80', + 'google.com', + 'pointerpointer.com', '192.168.1.123', '192.168.1.2:8080', '[::1]', @@ -149,28 +156,18 @@ describe('Validation', () => { tests.forEach((test) => { const headers = { host: test }; - if (!server.checkHost(headers)) { - throw new Error("Validation didn't pass"); + const isPublicHostname = test === options.public; + const isInAllowedHosts = options.allowedHosts.includes(test); + if (server.checkHost(headers)) { + if (!isPublicHostname && !isInAllowedHosts) + throw new Error("Validation didn't fail. It should"); + } else { + if (isPublicHostname || isInAllowedHosts) + throw new Error("Validation failed and it shouldn't"); } }); }); - it("should not allow hostnames that don't match options.public", () => { - const options = { - public: 'test.host:80' - }; - - const headers = { - host: 'test.hostname:80' - }; - - const server = new Server(compiler, options); - - if (server.checkHost(headers)) { - throw new Error("Validation didn't fail"); - } - }); - it('should allow urls with scheme for checking origin', () => { const options = { public: 'test.host:80' From dd92f3120ff5f7a25c38833f88d490bef8f0c4e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Requena=20L=C3=B3pez?= Date: Tue, 8 Jan 2019 22:52:59 +0100 Subject: [PATCH 3/3] fix eslint errors --- lib/Server.js | 1 - test/Validation.test.js | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index 7fa373d0fe..5e1f023f4c 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -12,7 +12,6 @@ const fs = require('fs'); const path = require('path'); -const ip = require('ip'); const tls = require('tls'); const url = require('url'); const http = require('http'); diff --git a/test/Validation.test.js b/test/Validation.test.js index 877579bcb0..acf33e0ac7 100644 --- a/test/Validation.test.js +++ b/test/Validation.test.js @@ -159,11 +159,14 @@ describe('Validation', () => { const isPublicHostname = test === options.public; const isInAllowedHosts = options.allowedHosts.includes(test); if (server.checkHost(headers)) { - if (!isPublicHostname && !isInAllowedHosts) + if (!isPublicHostname && !isInAllowedHosts) { throw new Error("Validation didn't fail. It should"); + } } else { - if (isPublicHostname || isInAllowedHosts) + // eslint-disable-next-line no-lonely-if + if (isPublicHostname || isInAllowedHosts) { throw new Error("Validation failed and it shouldn't"); + } } }); });