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

Add disableHostCheck to WebpackDevServer #932

Closed
mocheng opened this issue May 9, 2017 · 6 comments
Closed

Add disableHostCheck to WebpackDevServer #932

mocheng opened this issue May 9, 2017 · 6 comments
Milestone

Comments

@mocheng
Copy link
Contributor

mocheng commented May 9, 2017

Since webpack-dev-server 0.16.4, it requires webpack-dev-server to be initialized with host name according to webpack/webpack-dev-server#887 . Be default, webpack-dev-server only accepts requests with same host name passed in option. Even though localhost is in blacklist. This still brings some trouble in local development, because sometimes we'd like to access development environment from another device. In these cases, react-server is accessed with IP address or domain other than localhost.

The easies fix is to add disableHostCheck in options to WebpackDevServer in https://github.com/redfin/react-server/blob/v0.6.4/packages/react-server-cli/src/commands/start.js#L180-L188 . `

diff --git a/packages/react-server-cli/src/commands/start.js b/packages/react-server-cli/src/commands/start.js
index 33f8550..fa3fa9d 100644
--- a/packages/react-server-cli/src/commands/start.js
+++ b/packages/react-server-cli/src/commands/start.js
@@ -185,6 +185,7 @@ const startHotLoadJsServer = (compiler, port, bindIp, longTermCaching, httpsOpti
                key: httpsOptions ? httpsOptions.key : undefined,
                cert: httpsOptions ? httpsOptions.cert : undefined,
                ca: httpsOptions ? httpsOptions.ca : undefined,
+               disableHostCheck: true,
        });
        const serverStartedPromise = new Promise((resolve, reject) => {
                jsServer.listen(port, bindIp, (e) => {

HotLoadJsServer` tends to be only used in development. So, this change would not bring security issue.

@edmorley
Copy link

HotLoadJsServer` tends to be only used in development. So, this change would not bring security issue.

This unfortunately isn't true. The security issue here is a DNS rebinding attack which will still affect development - see the explanation in:
webpack/webpack-dev-server#887

@mocheng
Copy link
Contributor Author

mocheng commented May 16, 2017

@edmorley Thanks for the head-up. I re-read the DNS rebinding vulnerability and got it.

Hm~, then I have to update my local file in node_modules to workaround it.

@mocheng
Copy link
Contributor Author

mocheng commented May 16, 2017

But, even webpack-dev-server takes request with hostname localhost is fine (https://github.com/webpack/webpack-dev-server/blob/29578537c1ad38f29a445237c57a52c373183e75/lib/Server.js#L412 ), which is for better developer experience.

Without tweak on react-sever code, developer has to work on http://localhost or http://127.0.0.1. This might be enough. For example, local development depends on some cookie only available to xxx.com. It is preferred to develop on http://localhost.xxx.com so that it can access cookie in that domain. localhost.xxx.com can be resolved to be 127.0.0.1 in hosts file.

Is there anything we can do in react-server to make it work and secure?

@maddadder
Copy link

Did anyone come up with a solution to this?

@edmorley
Copy link

edmorley commented Oct 20, 2017

If it helps, recent versions of webpack-dev-server now auto-whitelist hostnames that are IP-address-like (eg 192.168.0.123 would not require any additional configuration to be permitted out of the box), plus allowedhosts now supports wildcards for subdomains:
https://webpack.js.org/configuration/dev-server/#devserver-allowedhosts

With those changes there should hopefully be very few cases that require the use of disableHostCheck.

@drewpc drewpc added this to the 1.0 milestone Apr 24, 2019
@drewpc
Copy link
Collaborator

drewpc commented Apr 30, 2019

No longer necessary due to #1003.

@drewpc drewpc closed this as completed Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants