Skip to content

Verify Host and Origin headers in dev server #10138

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

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from
Open

Conversation

devongovett
Copy link
Member

Previously we allowed all requests in the dev server, but this could potentially allow another website to access local source code.

This implements 3 mitigations:

  1. Validate the Origin header matches an allowed host. By default localhost and local IP addresses are allowed. The existing --host option can be specified to use a different host. Set Access-Control-Allow-Origin to only that origin instead of *.
  2. Validate the Origin header when a WebSocket connects. This allows localhost by default, or the --hmr-host/--host option.
  3. Validate the Host header to prevent DNS rebinding attacks.

One problem is that reverse proxy services such as Cloudflare Tunnels no longer work due to Host validation. Setting the --host option does not work because that also sets the network interface, and the cloudflare host is not valid in that situation. We would need to add an additional option to either disable host validation or provide additional allowed hosts. Unfortunately we don't have a great place to put this. Environment variable? New CLI option? Seems important to solve this one.

Another downside is if you use custom hostnames (e.g. configured via /etc/hosts), these are not allowed by default. You'd need to manually set the --host option now, and only one at a time can be used. Would be nice if these could automatically be added to the allowed hosts (e.g. could do a reverse DNS lookup for 127.0.0.1), but would this then be susceptible to DNS rebinding?

Unless we can solve the above items this is also technically a breaking change...

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

Successfully merging this pull request may close these issues.

1 participant