-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
The X-Forwarded-Host HTTP header is always trusted and is used in url_for #29893
Comments
I think a simple solution would be for |
I think the big problem is, this kind of response will be cached by rails if you enable page cache and use url_for in views. |
Looked into it a bit more and my comment above isn't necessary (as redirects won't be cached), so I don't think redirects are an actual issue. As far as I can tell, the mitigation technique above does work (however, it's not enabled by default). Without it, you can end up with cache poisoning (as you mention @ouabing). The same thing can be accomplished with the |
Thanks, everyone for your comments. @ankane Thanks for your input. You are right about For ease of reference, I've created a new heroku app, https://forwarded-host-demo-default.herokuapp.com/, that is running the code from the default-host branch (link to diff) of the demo app, which contains the It's running the same commit,
When I go through the steps above to capture a request for
the response is still
In ankane/secure_rails#4, you also suggested that we try to render
<!DOCTYPE html>
...
<pre>
HTTP_X_FORWARDED_HOST = evil.com
request.host = evil.com
root_url = https://forwarded-host-demo.herokuapp.com/
</pre>
<a href="/welcome/foo">redirect back to home page</a>
... If I make the same request to the app without the
<!DOCTYPE html>
...
<pre>
HTTP_X_FORWARDED_HOST = evil.com
request.host = evil.com
root_url = http://evil.com/
</pre>
<a href="/welcome/foo">redirect back to home page</a>
... So, to sum up, I think this shows that:
Finally, I'd also note that the other workaround I suggested, which was to strip the I'll also edit my comment above to make note of these points. Thanks again! |
This issue has been automatically marked as stale because it has not been commented on for at least three months. |
**Why**: To help prevent host header injection. See: https://github.com/ankane/secure_rails and rails/rails#29893
**Why**: To help prevent host header injection. See: https://github.com/ankane/secure_rails and rails/rails#29893
**Why**: To help prevent host header injection. See: https://github.com/ankane/secure_rails and rails/rails#29893
**Why**: To help prevent host header injection. See: https://github.com/ankane/secure_rails and rails/rails#29893
**Why**: To help prevent host header injection. See: https://github.com/ankane/secure_rails and rails/rails#29893
A vulnerability scanner just hit my site with this exploit. The redirect was implemented like |
Shouldn't this have a CVE number assigned to the Rails framework? If the Rails framework by default trusts the attacker for key functions, and that is not clearly documented on every function (e.g., "do not use"), then it seems like this deserves to be tracked as a vulnerability. |
I'm surprised that this is still an issue, that it still doesn't have a CVE, and that this assumption (that Rails can use client-supplied We've found that defining a whitelist of valid hosts works best, by blocking malicious clients setting either Include this in your ApplicationController:
|
You might find the following paper interesting - in it I explain in depth how the X-Forwarded-Host header can be used to exploit numerous real systems: https://portswigger.net/blog/practical-web-cache-poisoning Zend, Symfony and Drupal had very a similar issue, and resolved it with a coordinated security release. As such I'm a little surprised to find this has been languishing in a public issue tracker. |
We already have a bit of work-in-progress code to guard against basic host header attacks in #33145. If you folks have ideas how we can handle the |
So, in our environment, we usually overwrite all of the Host headers with the app's canonical host at the Load Balancer (nginx), but on the app I'm working on, we wanted for one single app to serve up the same content with two different configurations, depending on which URL you hit. So we disabled the Host Header overwriting config in our nginx server, and carefully configured the app to only pay attention to the request.headers['SERVER_NAME'] variable when deciding which configuration to serve up. This seems to be the recommended way to mitigate these kinds of attacks. But, from our vulnerability scanner, we are still vulnerable, and it seems to be the same issue as in this report. For me, the code that triggered this report from our vulnerability scanner was a redirect with a path helper, as I think others have reported:
I guess 302 redirects are required to include an absoluteURI, so even though I haven't explicitly invoked a URL helper, Rails decides that it must pull a URI from somewhere. I am getting a full URL (using X-Forwarded-Host header value eg. I implemented the middleware described at the top level of this thread to strip X-Forwarded-Host on the application, and I suspect this will solve it for our scanner, will know soon pending the next scan results. I've read that SERVER_NAME is not guaranteed to be defined, but in our configuration at least, Nginx will definitely define it, and we can use it for what we need to know (what app name was this requester using, or "what name am I called," from the allowed hosts defined in the nginx vhost.) We could probably configure Nginx to strip out X-Forwarded-Host too, for a solution that would not require adding a new middleware to each and every one of our Rails apps. I think the solution is highly specific to my particular front-end load balancer configuration. #33145 looks like a really well thought-out solution, I like it, but I'm not sure if there's another solution that would not require me to maintain a separate whitelist in my nginx vhost block's server_name directive, and another one in my Rails app. The advice I'm reading indicates that certain values like ServerName can be trusted if they are coming from your own front-end HTTP server. I'm not sure there's a way to generalize that for Rails apps that might not be running behind any front-end server, or behind a basic eg. ELB "Layer 4". |
Just ran into an issue where a missing HTTP_X_FORWARDED_HOST header prevents mounting engine routes. See sidekiq/sidekiq#4080 for details. |
I found a solution for mounting routes. I used to remove the HTTP_X_FORWARDED_HOST from the headers but if I just set it to the default host the routes are mounted. I suppose this also mitigates the security risk.
|
We ran into this issue as well and had to apply a solution similar to what others have recommended above, by implementing and applying a header sanitization filter in the form of a rack middleware. While that worked, the fact the
We could accomplish both by allowing a new configuration option that has a sensible default and allows overriding. Rails.application.config.action_controller.host_validation = [ "domain.com", "*.domain.com" ] Thoughts? I can submit a PR based on feedback. |
FYI re: I guess my assumption was if i had to add localhost to development env in order for it to work, i forsure needed to add my hosts to |
Now that rails 6 is out and provides |
If a user sets an X-Forwarded header in a request, this will in some cases override the Host header in a Rails app, resulting in URL redirection to a different website. If an attacker manages to poison a cache with such a header, users will be redirected to malicious sites. In rails/rails#29893, the resolution is to set config.hosts. Therefore setting this for all environments in this app.
If a user sets an X-Forwarded header in a request, this will in some cases override the Host header in a Rails app, resulting in URL redirection to a different website. If an attacker manages to poison a cache with such a header, users will be redirected to malicious sites. In rails/rails#29893, the resolution is to set config.hosts. Therefore setting this for all environments in this app.
If a user sets an X-Forwarded header in a request, this will in some cases override the Host header in a Rails app, resulting in URL redirection to a different website. If an attacker manages to poison a cache with such a header, users will be redirected to malicious sites. In rails/rails#29893, the resolution is to set config.hosts. Therefore setting this for all environments in this app.
If a user sets an X-Forwarded header in a request, this will in some cases override the Host header in a Rails app, resulting in URL redirection to a different website. If an attacker manages to poison a cache with such a header, users will be redirected to malicious sites. In rails/rails#29893, the resolution is to set config.hosts. Therefore setting this for all environments in this app.
If a user sets an X-Forwarded header in a request, this will in some cases override the Host header in a Rails app, resulting in URL redirection to a different website. If an attacker manages to poison a cache with such a header, users will be redirected to malicious sites. In rails/rails#29893, the resolution is to set config.hosts. Therefore setting this for all environments in this app.
If a user sets an X-Forwarded header in a request, this will in some cases override the Host header in a Rails app, resulting in URL redirection to a different website. If an attacker manages to poison a cache with such a header, users will be redirected to malicious sites. In rails/rails#29893, the resolution is to set config.hosts. Therefore setting this for all environments in this app.
… localhost or 127.0.0.1. Pretty sweet huh? The reason for this change is because I want to easily get to these healthcheck end points from localhost without having to get the health-check-token beforehand. Buttttt.... this addition has some issues. You can spoof the host header pretty easily: https://daniel.haxx.se/blog/2018/04/05/curl-another-host/ I thought about doing `return if !request.ssl? && request.local?` to check of the request was over https or not. Any localhost healthcheck we do directly to puma will be over http. Checking for TLS seemed like a logical next check, but since we force https on the edge and not necessarily from the edge to Puma the Host header can still be spoofed to get at these end points. More info on host header attacks and what Rails 6 is doing about it: rails/rails#29893 While I do think it is a good idea to protect these healthcheck end points from the outside world, I do want to be able to get to them easily locally. WWYD?
…th-check-token (#8231) * This change allows access to api/health_checks if you are coming from localhost or 127.0.0.1. Pretty sweet huh? The reason for this change is because I want to easily get to these healthcheck end points from localhost without having to get the health-check-token beforehand. Buttttt.... this addition has some issues. You can spoof the host header pretty easily: https://daniel.haxx.se/blog/2018/04/05/curl-another-host/ I thought about doing `return if !request.ssl? && request.local?` to check of the request was over https or not. Any localhost healthcheck we do directly to puma will be over http. Checking for TLS seemed like a logical next check, but since we force https on the edge and not necessarily from the edge to Puma the Host header can still be spoofed to get at these end points. More info on host header attacks and what Rails 6 is doing about it: rails/rails#29893 While I do think it is a good idea to protect these healthcheck end points from the outside world, I do want to be able to get to them easily locally. WWYD? * Fix Health Check spec by stubbing request Co-authored-by: mstruve <mollylbs@gmail.com>
avoid user control of the host header used in rails to create links that can direct to malicous URLs, rails/rails#29893
pass the server_name to the proxy's request to avoid malicious user controlled links, rails/rails#29893
* Remove X-Forwarded-Host header Carrying over zooniverse/operations#283 * use correct staging server_name * manually set the X-Forwarded-Host to our servername avoid user control of the host header used in rails to create links that can direct to malicous URLs, rails/rails#29893 * split out server blocks to provide unique server_names pass the server_name to the proxy's request to avoid malicious user controlled links, rails/rails#29893 Co-authored-by: Campbell Allen <campbell.allen@gmail.com>
Background
This has been reported twice on the rails HackerOne program, and the recommendation (from Jeremy) was to open a GitHub issue:
Steps to Reproduce
We're going to simulate a host header attack on a sample application. The sample application is a vanilla rails 5.1.2 application with a home page and one route added,
foo
, to redirect back to the home page. The code is available here: https://github.com/jdleesmiller/forwarded_host_demoVisit http://forwarded-host-demo.herokuapp.com/ (it may need some time to boot up)
Open the network tools in your browser (I used Chrome) and tick the option to preserve requests.
Click the 'redirect back to home page' link. You are redirected to the home page.
Copy the corresponding request for
/foo
as a cURL command from the browser's network tools (right click, Copy, Copy as cURL).To simulate a host header attack, paste the curl command into a terminal and add
-H 'X-Forwarded-Host: evil.com'
. For example, for one of my requests:Expected Result
User is redirected to the home page:
Observed Result
User is redirected to the home page on evil.com:
(You can also verify the HTTP Location header for the redirect is set to
http://evil.com
by passing-i
to curl.)System configuration
Rails version: 5.1.2
Ruby version: 2.4.0p0
Analysis
As I understand it, rails uses the
X-Forwarded-Host
HTTP header in preference to theHost
HTTP header in ActionDispatch::Http::URL to computerequest.host
.request.host
is in turn used inurl_for
.Impact
This makes it very easy to create an open redirect in a rails application, in addition to creating many opportunities for reflection of URLs controlled by an attacker, since it affects all of rails's URL helpers through
url_for
.To exploit it, the attacker needs to be able to inject the
X-Forwarded-Host
header, which can be accomplished by cache poisoning.There is at least one public bug report on HackerOne that demonstrates an attack vector, https://hackerone.com/reports/487.
It seems that GitLab tried to patch this problem in https://gitlab.com/gitlab-org/gitlab-ce/issues/17877 by removing the header in their NGINX layer. However, this caused a number of problems, and they had to remove the patch. According to the issue, they are going to try again with a whitelist of hosts.
Whether the
X-Forwarded-Host
header is under the user's control does depend on the upstream proxy configuration. On heroku, the steps to reproduce above with my sample app show that it is under user control, so that accounts for a large number of rails applications.Mitigation
This list of best practices https://github.com/ankane/secure_rails says that you should set
to avoid host injection, but when I tested it,
it had no effect(edit: it is a partial fix; see #29893 (comment)). (I tried it on this branch: https://github.com/jdleesmiller/forwarded_host_demo/tree/default-host .)I think the(see #29893 (comment))request.host
takes precedence over the default setting, which is only used when operating without a request, for example in a background worker.Express.js has a
trust proxy
setting that, among other things, determines whether the app will trustX-Forwarded-Host
to set the hostname. I could not find any similar option for rails; rails has good handling of IP spoofing withX-Forwarded-For
, but I cannot see any countermeasures againstX-Forwarded-Host
spoofing.If the header is not required, removing it appears to solve the problem. One way to do this is with a small piece of Rack middleware in
config/application.rb
:(Edit: This gem also implements the same approach: https://github.com/pusher/rack-headers_filter .)
This is the approach I'm trialling on my app, and so far it works OK.
Jeremy on HackerOne suggested:
I'm not at this point what the best approach is, so I've opted to open an issue for discussion.
I hope that's all clear. Let me know if you have any questions.
The text was updated successfully, but these errors were encountered: