bidi icon indicating copy to clipboard operation
bidi copied to clipboard

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

Open danielcompton opened this issue 6 years ago • 4 comments

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.

danielcompton avatar Jul 08 '18 22:07 danielcompton

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 avatar Jul 09 '18 05:07 SevereOverfl0w

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.

SevereOverfl0w avatar Jul 09 '18 06:07 SevereOverfl0w

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?

danielcompton avatar Jul 09 '18 08:07 danielcompton

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

SevereOverfl0w avatar Jul 09 '18 14:07 SevereOverfl0w