Description
Is there an existing issue for this?
- I have searched the existing issues
OS/Web Information
N/A
Steps to Reproduce
You can simulate with this NGINX config:
proxy_set_header X-Forwarded-Host $host:$server_port;
Expected
If your origin is https://domain.tld
and your host is domain.tld:443
the check should pass. Same for http://domain.tld
and domain.tld:80
.
Also I think NGINX's $host
actually does not include ports so it will fail if your config only has $host
and if you host on a port other than 443 and 80 since you would get an origin like https://domain.tld:8080
and the host would be domain.tld
.
To fix the first we could just check the protocol on the origin and then add/remove 443 or 80.
For the second we could ignore the port altogether since I think the vulnerability does not happen across ports...
But I am not sure we should do anything; maybe the correct course of action is to edit the proxy config so the host and origin headers match. I have looked at other software but they all seem to do exact matches without messing around with the port. We could just edit the documentation to use $http_host
.
Actual
The origin and domain are matched exactly so they do not match. Ends up causing the web sockets to fail with 1006.
Logs
No response
Screenshot/Video
No response
Does this issue happen in VS Code or GitHub Codespaces?
- I cannot reproduce this in VS Code.I cannot reproduce this in GitHub Codespaces.
Are you accessing code-server over HTTPS?
- I am using HTTPS.
Notes
Might be causing issues reported in #6023 and #6064 as well.
And possibly #6014
Activity
[-]Account for hosts with 443 and 80 in origin check[/-][+]Account for hosts with ports in origin check[/+][-]Account for hosts with ports in origin check[/-][+]Account for hosts with/witohut ports in origin check[/+][-]Account for hosts with/witohut ports in origin check[/-][+]Account for hosts with/without ports in origin check[/+]smerschjohann commentedon May 10, 2023
I just stumbled upon this issue. The X-Forwarded-Host describes the host domain, so it should not contain the port. If you want to give information about the protocol (http, https) there is another header for that: X-Forwarded-Proto. The port itself is typically irrelevant.
code-asher commentedon May 11, 2023
Oh thanks! Interesting, I never noticed it explicitly excluded the port. Seems odd since
Host
itself does include the port.I believe the attack only occurs across sub-domains and not across ports since browsers would treat the latter as separate origins so with everything considered sounds like we can just ignore the port.