Possible issue with client IP in X-Forwarded-For when checking #cloudflare?
#161
Comments
geoffharcourt
added a commit
to geoffharcourt/cloudflare-rails
that referenced
this issue
Oct 4, 2024
In modosc#149 we got the extremely helpful `Request#cloudflare?` method which can assist with checking if a request has been proxied through Cloudflare. The method checks that there's an unbroken chain of trust from the `REMOTE_ADDR` (if it exists) through forwarded IPs that only includes trusted internal proxies and Cloudflare IP addresses. Each proxy in the chain is supposed to append the IP of the traffic it received to the *right side* of `X-Forwarded-For`. This means that after evaluating `REMOTE_ADDR` we should be looking at addresses in `X-Forwarded-For` starting on the right side, not on the left. https://github.com/modosc/cloudflare-rails/blob/311875eef57862ecf050fb87415c56ea4823486a/lib/cloudflare_rails/check_trusted_proxies.rb#L23-L25 This ordering means that if a request has the original client as the first address in `X-Forwarded-For` that we'll incorrectly reject it. It also means that an attacker could craft a forged `X-Forwarded-For` that starts with a Cloudflare IP and smuggle in a request that appears to be valid to the method's logic. The fix here is to continue to look at `REMOTE_ADDR` first but then to look at `X-Forwarded-For` in right-to-left order until we run out of trusted IP addresses (whether internal proxies or Cloudflare IPs). Fix modosc#161
|
We tried this method on Heroku, and Heroku adds their own (private IP) load balancer as |
|
Hi @mamhoff! That was closed by accident. Thank you for flagging. |
modosc
pushed a commit
that referenced
this issue
Oct 7, 2024
In #149 we got the extremely helpful `Request#cloudflare?` method which can assist with checking if a request has been proxied through Cloudflare. The method checks that there's an unbroken chain of trust from the `REMOTE_ADDR` (if it exists) through forwarded IPs that only includes trusted internal proxies and Cloudflare IP addresses. Each proxy in the chain is supposed to append the IP of the traffic it received to the *right side* of `X-Forwarded-For`. This means that after evaluating `REMOTE_ADDR` we should be looking at addresses in `X-Forwarded-For` starting on the right side, not on the left. https://github.com/modosc/cloudflare-rails/blob/311875eef57862ecf050fb87415c56ea4823486a/lib/cloudflare_rails/check_trusted_proxies.rb#L23-L25 This ordering means that if a request has the original client as the first address in `X-Forwarded-For` that we'll incorrectly reject it. It also means that an attacker could craft a forged `X-Forwarded-For` that starts with a Cloudflare IP and smuggle in a request that appears to be valid to the method's logic. The fix here is to continue to look at `REMOTE_ADDR` first but then to look at `X-Forwarded-For` in right-to-left order until we run out of trusted IP addresses (whether internal proxies or Cloudflare IPs). Fix #161
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We're looking at a new environment and while troubleshooting an issue, I think I found an issue in the Cloudflare request check. If this is actually a problem, it's not hole, it breaks in a way that's restrictive rather than permissive. However, I think it might fail for a not-uncommon setup.
A request from the client at 1.2.3.4 goes through Cloudflare to our ingress to our Puma server. We end up with:
REMOTE_ADDR: trusted proxyX_FORWARDED_FOR: 1.2.3.4,a-cloudflare-ip#cloudflare?checks if the request passed through Cloudflare by taking the remote address(es) and the IPs fromX-Forwarded-For, chaining them together, and then taking elements from the left side until an untrusted IP address is encountered. However, if the client IP is on the left side ofX-Forwarded-For(which seems allowable under the spec), we'll reject the request. Since appends happen to the right and we can only trust the right-most element of X-Forwarded-For, shouldn't we be evaluating from that direction and not from the left?The text was updated successfully, but these errors were encountered: