From da4800420cd377960b6f20b94b97aa49e4bf94e0 Mon Sep 17 00:00:00 2001
From: Nico <ncls.ayala@gmail.com>
Date: Sat, 1 Jul 2023 06:45:14 -0300
Subject: [PATCH] Vulnerability fix: validate return_to param using
 request.host (#3627)

* Use starts_with?

Add specs

* Fix linter issues

* Fix linter issues. Disable metric ClassLength cop on config file.

* Use base_url to check return_to param

* Fix linter issues

* Consider redirect to paths. Fix linter issues.

* Address feedback

---------

Co-authored-by: Steven Chau <steven.chau@himaxwell.com>
---
 .../rails_admin/main_controller.rb            |  6 +-
 .../rails_admin/main_controller_spec.rb       | 66 +++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/app/controllers/rails_admin/main_controller.rb b/app/controllers/rails_admin/main_controller.rb
index 5968bb7bec..af5a69aba3 100644
--- a/app/controllers/rails_admin/main_controller.rb
+++ b/app/controllers/rails_admin/main_controller.rb
@@ -56,7 +56,11 @@ def respond_to_missing?(sym, include_private)
     end
 
     def back_or_index
-      params[:return_to].presence && params[:return_to].include?(request.host) && (params[:return_to] != request.fullpath) ? params[:return_to] : index_path
+      allowed_return_to?(params[:return_to].to_s) ? params[:return_to] : index_path
+    end
+
+    def allowed_return_to?(url)
+      url != request.fullpath && url.start_with?(request.base_url, '/') && !url.start_with?('//')
     end
 
     def get_sort_hash(model_config)
diff --git a/spec/controllers/rails_admin/main_controller_spec.rb b/spec/controllers/rails_admin/main_controller_spec.rb
index 82d4f2cc28..aff2eaeb47 100644
--- a/spec/controllers/rails_admin/main_controller_spec.rb
+++ b/spec/controllers/rails_admin/main_controller_spec.rb
@@ -470,4 +470,70 @@ def get(action, params)
       )
     end
   end
+
+  describe 'back_or_index' do
+    before do
+      allow(controller).to receive(:index_path).and_return(index_path)
+    end
+
+    let(:index_path) { '/' }
+
+    it 'returns back to index when return_to is not defined' do
+      controller.params = {}
+      expect(controller.send(:back_or_index)).to eq(index_path)
+    end
+
+    it 'returns back to return_to url when it starts with same protocol and host' do
+      return_to_url = "http://#{request.host}/teams"
+      controller.params = {return_to: return_to_url}
+      expect(controller.send(:back_or_index)).to eq(return_to_url)
+    end
+
+    it 'returns back to return_to url when it contains a path' do
+      return_to_url = '/teams'
+      controller.params = {return_to: return_to_url}
+      expect(controller.send(:back_or_index)).to eq(return_to_url)
+    end
+
+    it 'returns back to index path when return_to path does not start with slash' do
+      return_to_url = 'teams'
+      controller.params = {return_to: return_to_url}
+      expect(controller.send(:back_or_index)).to eq(index_path)
+    end
+
+    it 'returns back to index path when return_to url does not start with full protocol' do
+      return_to_url = "#{request.host}/teams"
+      controller.params = {return_to: return_to_url}
+      expect(controller.send(:back_or_index)).to eq(index_path)
+    end
+
+    it 'returns back to index path when return_to url starts with double slash' do
+      return_to_url = "//#{request.host}/teams"
+      controller.params = {return_to: return_to_url}
+      expect(controller.send(:back_or_index)).to eq(index_path)
+    end
+
+    it 'returns back to index path when return_to url starts with tripple slash' do
+      return_to_url = "///#{request.host}/teams"
+      controller.params = {return_to: return_to_url}
+      expect(controller.send(:back_or_index)).to eq(index_path)
+    end
+
+    it 'returns back to index path when return_to url does not have host' do
+      return_to_url = 'http:///teams'
+      controller.params = {return_to: return_to_url}
+      expect(controller.send(:back_or_index)).to eq(index_path)
+    end
+
+    it 'returns back to index path when return_to url starts with different protocol' do
+      return_to_url = "other://#{request.host}/teams"
+      controller.params = {return_to: return_to_url}
+      expect(controller.send(:back_or_index)).to eq(index_path)
+    end
+
+    it 'returns back to index path when return_to does not start with the same protocol and host' do
+      controller.params = {return_to: "http://google.com?#{request.host}"}
+      expect(controller.send(:back_or_index)).to eq(index_path)
+    end
+  end
 end