mastercactapus/caddy-proxyprotocol

Denial of service vulnerability with invalid v2 PROXY data

Closed this issue · 7 comments

cpu commented

I opened an issue describing a DoS vulnerability in the github.com/mastercactapus/proxyprotocol package used by this plugin: mastercactapus/proxyprotocol#1

This code is the only consumer of the package I was able to find with light googling/github searching.

Since fixing the mastercactapus/proxyprotocol parsing bug will require an updated version of this plugin I wanted to file an issue here as well so it doesn't fall through the cracks.

Configuring the plugin with a source address filter is one potential mitigation in the short term.

The documentation would greatly benefit from strong language encouraging the use of a source address filter in all circumstances since above-and-beyond the current bug omitting such a filter will allow any client to spoof the IP metadata processed by the plugin. E.g. Apache's equivalent documentation says:

It is critical to only enable this behavior from intermediate hosts (proxies, etc) which are trusted by this server, since it is trivial for the remote useragent to impersonate another useragent.

cpu commented

Here’s a quick standalone way to check if the installed Caddy server w/ the caddy-proxyprotocol plugin is vulnerable:

printf "\x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A\x21\x12\x00\x00" | nc

If the server is configured with proxyprotocol the request generated with the above command will terminate the Caddy process on the server.

cpu commented

MITRE assigned this CVE-2019-14243.

Thanks for this, especially the quick vuln. test and being so thorough with information.


As a quick aside, for reference from the release notes:

The scope of this vulnerability is limited to cases where an attacker is able to send traffic directly to the server, by bypassing the load balancer entirely. Since that's the sole purpose of this plugin (to be used behind a LB/proxy) hopefully that means real-world impact is minimal.

Also worth noting: all HTTP/non-PROXY traffic is rejected with a 400 status code, as the header is always required for enabled sources (requiring proper config).

If a 200 response (or anything other than 400) is returned to curl, for example, then that URL/address should be safe, as that means it's already going through a proxy providing a valid header or that it is disabled for that source.

Closing, new version is released with fix -- validated build from caddyserver.com

cpu commented

Closing, new version is released with fix -- validated build from caddyserver.com

@mastercactapus Great work on the quick resolution. Thank you!!

The scope of this vulnerability is limited to cases where an attacker is able to send traffic directly to the server, by bypassing the load balancer entirely. Since that's the sole purpose of this plugin (to be used behind a LB/proxy) hopefully that means real-world impact is minimal.

100% in agreement with you there. 👍

@mastercactapus Do you control the content of the docs on https://caddyserver.com/docs/http.proxyprotocol or is that handled in the upstream Caddy project? Is there somewhere I could open a PR with some suggested language to help emphasize the need for restricting access to the PROXY interface?

I think it would be sensible to explicitly mention what the risks are if you don't use any sort of source address filtering or additional firewall rules. WDYT?

I do, and that would be a good idea. It's not in a repo but there's a UI I can make changes in.

How about something like this (adapted from the Apache message):

**Note** It is important to filter to only trusted sources (e.g. proxies, LB), 
since it is trivial to spoof the source address otherwise.

Basically, same rules as X-Forwarded-For. You think that would be adequate?

cpu commented

@mastercactapus That sounds good to me 👍 Thanks!