Support for the X-Forwarded-Host

[A----]

Fixes #2877

[drzraf]

• Contrary to SERVER_NAME which is server provided,
• ... and HTTP_HOST which is user-provided but often defacto filtered-down by httpd configuration,
• X_FORWARDED_HOST is under full control of the visitors (except in the rare situations where httpd is configured to deal with this header)

Is user-control of the $hostname variable (and the implication for callers) safe?

[mahagr]

We are filtering hostname for illegal letters. We are using host internally to load the correct site configuration and checking against referrer if it's coming from the same site. Maybe something else as well?

[drzraf]

Sorry, to worry. I'm maybe a bit over-reactive, but I'm trying to figure-out an adversary use of X-Forwarded-Host: 127.0.0.1 (valid value) or a way to inventory multi-sites setup or just tweak some functions' behavior.

There are quite a lot of references to new Uri() or Uri::getCurrentUri() and some of them make actual use of the $host component for possibly critical guesses.

Just one situation as an example (significant or not?):

// system/src/Grav/Common/Page/Markdown/Excerpts.php : processImageExcerpt()
 $local_file = isset($url_parts['path'])
                && (empty($url_parts['scheme']) || in_array($url_parts['scheme'], ['http', 'https'], true))
                && (empty($url_parts['host']) || $url_parts['host'] === $grav['uri']->host());

With X-Forwarded-Host, an attacker could easily trick this function to make it think the media within a given markdown input is local while it's actually not.

Here there is no subsequent file_get_contents() or fopen(), but we can imagine how, in the future, this could be mistakenly introduced somewhere in the codebase, and easily open unnoticed doors (including within themes/plugins).

[mahagr]

@rhukster I think this is a valid concern and can be used for attacks. We cannot trust X_FORWARDED_HOST which means that it should not be used for anything.

https://www.skeletonscribe.net/2013/05/practical-http-host-header-attacks.html

Looks like this feature is disabled by default in most CMSes.

[rhukster]

I concur with you guys. I should of done more research before approving. It will be removed in next release.

[mahagr]

@rhukster Can we make it configurable and optional instead? I think that in the configurations where it is used, it will be always set by a nearby server/firewall.

[rhukster]

I've added configuration options for all of these in system.yaml but only 'host' is disabled by default so it maintains backwards compatibility:

http_x_forwarded:                                # Configuration options for the various HTTP_X_FORWARD headers
  protocol: true
  host: false
  port: true
  ip: true