Skip to content
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

X-Forwarded-Host is used over Host, can lead to security issues #186

Open
danielcompton opened this issue Jul 8, 2018 · 4 comments
Open

Comments

@danielcompton
Copy link

danielcompton commented Jul 8, 2018

I got a security report recently that boiled down to Bidi using the X-Forwarded-Host value over the Host value.

https://github.com/juxt/bidi/blob/master/src/bidi/vhosts.clj#L94-L98

This was added in 02d8f68.

You can see more descriptions of this host header attack here: https://hackerone.com/reports/123078, https://www.acunetix.com/blog/articles/automated-detection-of-host-header-attacks/, https://www.skeletonscribe.net/2013/05/practical-http-host-header-attacks.html, rails/rails#29893. Briefly, the attack consists of poisoning an upstream cache to use the attacker supplied X-Forwarded-Host header values.

It is reasonably well known that you need to be careful with the Host header and make sure that it is a value that you expect, as it is user supplied. However I didn't realise that I also needed to take the same care with the X-Forwarded-Host header.

One fix for this would be to make the handling of X-Forwarded-Host opt-in. It seems to be fairly rarely used. Another option is just to point out in the documentation this behaviour, though that is unlikely to be as effective.

@danielcompton danielcompton changed the title X-Forwarded-Host can contain untrusted values but is used over Host X-Forwarded-Host is used over Host, can lead to security issues Jul 8, 2018
@SevereOverfl0w
Copy link
Contributor

I'm a little bit confused. While trusting those headers is bad, bidi acts as a whitelist system, and that effectively prevents this doesn't it?

@SevereOverfl0w
Copy link
Contributor

I think I see the problem. Bidi has its own concept of what the actual host is, and you're reading the host header directly elsewhere in your application (for what?) and that could be set to evil.com.

@danielcompton
Copy link
Author

danielcompton commented Jul 9, 2018

I was using yada.context/url-for to create a URL for a link. They are internal links, so I could just as easily have used path-for.

I'm using :* as the host as I had issues with specifying it explicitly (don't remember what now). If the :host was specified in the vhosts-model, would that avoid the issues here?

@SevereOverfl0w
Copy link
Contributor

Ah, I hadn't considered those functionalities together. I don't think :* is very safe then. Specifying a host should alleviate this.

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

No branches or pull requests

2 participants