Ignore invalid trusted proxies in X-Forwarded-For headerreject-invalid-trusted-proxies

Certain reverse proxies can send invalid IP addresses in the X-Forwarded-For header
For example, Apache can send (null).

Closes #20194

3 files changed, 16 insertions, 3 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 971a709e4c1..973d1ae886c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -8,7 +8,8 @@ v 8.11.0 (unreleased)
   - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska)
 
 v 8.10.1 (unreleased)
- - Fix bug where replies to commit notes displayed in the MR discussion tab wouldn't show up on the commit page
+  - Ignore invalid trusted proxies in X-Forwarded-For header
+  - Fix bug where replies to commit notes displayed in the MR discussion tab wouldn't show up on the commit page
 
 v 8.10.0
   - Fix profile activity heatmap to show correct day name (eanplatter)
diff --git a/config/initializers/trusted_proxies.rb b/config/initializers/trusted_proxies.rb
index df4a933e22f..30770b71e24 100644
--- a/config/initializers/trusted_proxies.rb
+++ b/config/initializers/trusted_proxies.rb
@@ -11,6 +11,12 @@ module Rack
   end
 end
 
+gitlab_trusted_proxies = Array(Gitlab.config.gitlab.trusted_proxies).map do |proxy|
+  begin
+    IPAddr.new(proxy)
+  rescue IPAddr::InvalidAddressError
+  end
+end.compact
+
 Rails.application.config.action_dispatch.trusted_proxies = (
-  [ '127.0.0.1', '::1' ] + Array(Gitlab.config.gitlab.trusted_proxies)
-).map { |proxy| IPAddr.new(proxy) }
+  [ '127.0.0.1', '::1' ] + gitlab_trusted_proxies)
diff --git a/spec/initializers/trusted_proxies_spec.rb b/spec/initializers/trusted_proxies_spec.rb
index 14c8df954a6..52d5a7dffc9 100644
--- a/spec/initializers/trusted_proxies_spec.rb
+++ b/spec/initializers/trusted_proxies_spec.rb
@@ -17,6 +17,12 @@ describe 'trusted_proxies', lib: true do
       expect(request.remote_ip).to eq('10.1.5.89')
       expect(request.ip).to eq('10.1.5.89')
     end
+
+    it 'filters out bad values' do
+      request = stub_request('HTTP_X_FORWARDED_FOR' => '(null), 10.1.5.89')
+      expect(request.remote_ip).to eq('10.1.5.89')
+      expect(request.ip).to eq('10.1.5.89')
+    end
   end
 
   context 'with private IP ranges added' do