Skip to content

Commit da48004

Browse files
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>
1 parent f06df8f commit da48004

File tree

2 files changed

+71
-1
lines changed

2 files changed

+71
-1
lines changed

app/controllers/rails_admin/main_controller.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ def respond_to_missing?(sym, include_private)
5656
end
5757

5858
def back_or_index
59-
params[:return_to].presence && params[:return_to].include?(request.host) && (params[:return_to] != request.fullpath) ? params[:return_to] : index_path
59+
allowed_return_to?(params[:return_to].to_s) ? params[:return_to] : index_path
60+
end
61+
62+
def allowed_return_to?(url)
63+
url != request.fullpath && url.start_with?(request.base_url, '/') && !url.start_with?('//')
6064
end
6165

6266
def get_sort_hash(model_config)

spec/controllers/rails_admin/main_controller_spec.rb

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,4 +470,70 @@ def get(action, params)
470470
)
471471
end
472472
end
473+
474+
describe 'back_or_index' do
475+
before do
476+
allow(controller).to receive(:index_path).and_return(index_path)
477+
end
478+
479+
let(:index_path) { '/' }
480+
481+
it 'returns back to index when return_to is not defined' do
482+
controller.params = {}
483+
expect(controller.send(:back_or_index)).to eq(index_path)
484+
end
485+
486+
it 'returns back to return_to url when it starts with same protocol and host' do
487+
return_to_url = "http://#{request.host}/teams"
488+
controller.params = {return_to: return_to_url}
489+
expect(controller.send(:back_or_index)).to eq(return_to_url)
490+
end
491+
492+
it 'returns back to return_to url when it contains a path' do
493+
return_to_url = '/teams'
494+
controller.params = {return_to: return_to_url}
495+
expect(controller.send(:back_or_index)).to eq(return_to_url)
496+
end
497+
498+
it 'returns back to index path when return_to path does not start with slash' do
499+
return_to_url = 'teams'
500+
controller.params = {return_to: return_to_url}
501+
expect(controller.send(:back_or_index)).to eq(index_path)
502+
end
503+
504+
it 'returns back to index path when return_to url does not start with full protocol' do
505+
return_to_url = "#{request.host}/teams"
506+
controller.params = {return_to: return_to_url}
507+
expect(controller.send(:back_or_index)).to eq(index_path)
508+
end
509+
510+
it 'returns back to index path when return_to url starts with double slash' do
511+
return_to_url = "//#{request.host}/teams"
512+
controller.params = {return_to: return_to_url}
513+
expect(controller.send(:back_or_index)).to eq(index_path)
514+
end
515+
516+
it 'returns back to index path when return_to url starts with tripple slash' do
517+
return_to_url = "///#{request.host}/teams"
518+
controller.params = {return_to: return_to_url}
519+
expect(controller.send(:back_or_index)).to eq(index_path)
520+
end
521+
522+
it 'returns back to index path when return_to url does not have host' do
523+
return_to_url = 'http:///teams'
524+
controller.params = {return_to: return_to_url}
525+
expect(controller.send(:back_or_index)).to eq(index_path)
526+
end
527+
528+
it 'returns back to index path when return_to url starts with different protocol' do
529+
return_to_url = "other://#{request.host}/teams"
530+
controller.params = {return_to: return_to_url}
531+
expect(controller.send(:back_or_index)).to eq(index_path)
532+
end
533+
534+
it 'returns back to index path when return_to does not start with the same protocol and host' do
535+
controller.params = {return_to: "http://google.com?#{request.host}"}
536+
expect(controller.send(:back_or_index)).to eq(index_path)
537+
end
538+
end
473539
end

0 commit comments

Comments
 (0)