search

juxt / bidi

Bidirectional URI routing
MIT License
992 stars 92 forks source link

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

Open danielcompton opened 6 years ago

danielcompton commented 6 years ago

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 https://github.com/juxt/bidi/commit/02d8f68da68c99697bbd63ea4018647834b79687.

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, https://github.com/rails/rails/issues/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.

SevereOverfl0w commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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