From 8ab0ca7d64b9f7ec9d658d9a90eae1120ccef117 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 30 May 2024 14:24:29 +0200 Subject: [PATCH] Merge pull request from GHSA-c2r5-cfqr-c553 * Add hardening monkey-patch to prevent IP spoofing on misconfigured installations * Remove rack-attack safelist --- config/application.rb | 1 + config/initializers/rack_attack.rb | 4 -- lib/action_dispatch/remote_ip_extensions.rb | 72 +++++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 lib/action_dispatch/remote_ip_extensions.rb diff --git a/config/application.rb b/config/application.rb index 2a62c37e8..c2a63c869 100644 --- a/config/application.rb +++ b/config/application.rb @@ -48,6 +48,7 @@ require_relative '../lib/chewy/strategy/bypass_with_warning' require_relative '../lib/webpacker/manifest_extensions' require_relative '../lib/webpacker/helper_extensions' require_relative '../lib/rails/engine_extensions' +require_relative '../lib/action_dispatch/remote_ip_extensions' require_relative '../lib/active_record/database_tasks_extensions' require_relative '../lib/active_record/batches' require_relative '../lib/simple_navigation/item_extensions' diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index 426b14b25..962f02994 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -62,10 +62,6 @@ class Rack::Attack end end - Rack::Attack.safelist('allow from localhost') do |req| - req.remote_ip == '127.0.0.1' || req.remote_ip == '::1' - end - Rack::Attack.blocklist('deny from blocklist') do |req| IpBlock.blocked?(req.remote_ip) end diff --git a/lib/action_dispatch/remote_ip_extensions.rb b/lib/action_dispatch/remote_ip_extensions.rb new file mode 100644 index 000000000..e5c48bf3c --- /dev/null +++ b/lib/action_dispatch/remote_ip_extensions.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +# Mastodon is not made to be directly accessed without a reverse proxy. +# This monkey-patch prevents remote IP address spoofing when being accessed +# directly. +# +# See PR: https://github.com/rails/rails/pull/51610 + +# In addition to the PR above, it also raises an error if a request with +# `X-Forwarded-For` or `Client-Ip` comes directly from a client without +# going through a trusted proxy. + +# rubocop:disable all -- This is a mostly vendored file + +module ActionDispatch + class RemoteIp + module GetIpExtensions + def calculate_ip + # Set by the Rack web server, this is a single value. + remote_addr = ips_from(@req.remote_addr).last + + # Could be a CSV list and/or repeated headers that were concatenated. + client_ips = ips_from(@req.client_ip).reverse! + forwarded_ips = ips_from(@req.x_forwarded_for).reverse! + + # `Client-Ip` and `X-Forwarded-For` should not, generally, both be set. If they + # are both set, it means that either: + # + # 1) This request passed through two proxies with incompatible IP header + # conventions. + # + # 2) The client passed one of `Client-Ip` or `X-Forwarded-For` + # (whichever the proxy servers weren't using) themselves. + # + # Either way, there is no way for us to determine which header is the right one + # after the fact. Since we have no idea, if we are concerned about IP spoofing + # we need to give up and explode. (If you're not concerned about IP spoofing you + # can turn the `ip_spoofing_check` option off.) + should_check_ip = @check_ip && client_ips.last && forwarded_ips.last + if should_check_ip && !forwarded_ips.include?(client_ips.last) + # We don't know which came from the proxy, and which from the user + raise IpSpoofAttackError, "IP spoofing attack?! " \ + "HTTP_CLIENT_IP=#{@req.client_ip.inspect} " \ + "HTTP_X_FORWARDED_FOR=#{@req.x_forwarded_for.inspect}" + end + + # NOTE: Mastodon addition to make sure we don't get requests from a non-trusted client + if @check_ip && (forwarded_ips.last || client_ips.last) && !@proxies.any? { |proxy| proxy === remote_addr } + raise IpSpoofAttackError, "IP spoofing attack?! client #{remote_addr} is not a trusted proxy " \ + "HTTP_CLIENT_IP=#{@req.client_ip.inspect} " \ + "HTTP_X_FORWARDED_FOR=#{@req.x_forwarded_for.inspect}" + end + + # We assume these things about the IP headers: + # + # - X-Forwarded-For will be a list of IPs, one per proxy, or blank + # - Client-Ip is propagated from the outermost proxy, or is blank + # - REMOTE_ADDR will be the IP that made the request to Rack + ips = forwarded_ips + client_ips + ips.compact! + + # If every single IP option is in the trusted list, return the IP that's + # furthest away + filter_proxies([remote_addr] + ips).first || ips.last || remote_addr + end + end + end +end + +ActionDispatch::RemoteIp::GetIp.prepend(ActionDispatch::RemoteIp::GetIpExtensions) + +# rubocop:enable all