From 991b17a0321235aaa86026ebf621f0f183adb017 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Mon, 22 Feb 2021 17:49:08 -0500 Subject: [PATCH] [ci skip] Note dangerous use of redirect_to --- actionpack/lib/action_controller/metal/redirecting.rb | 4 ++++ guides/source/security.md | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 3919f71bfc..512ca12b12 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -54,7 +54,11 @@ module ActionController # # Statements after +redirect_to+ in our controller get executed, so +redirect_to+ doesn't stop the execution of the function. # To terminate the execution of the function immediately after the +redirect_to+, use return. + # # redirect_to post_url(@post) and return + # + # Passing user input directly into +redirect_to+ is considered dangerous (eg. `redirect_to(params[:location])`). + # Always use regular expressions or a permitted list when redirecting to a user specified location. def redirect_to(options = {}, response_options = {}) raise ActionControllerError.new("Cannot redirect to nil!") unless options raise AbstractController::DoubleRenderError if response_body diff --git a/guides/source/security.md b/guides/source/security.md index b2953c1127..15f536d584 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -342,7 +342,7 @@ This will redirect the user to the main action if they tried to access a legacy http://www.example.com/site/legacy?param1=xy¶m2=23&host=www.attacker.com ``` -If it is at the end of the URL it will hardly be noticed and redirects the user to the attacker.com host. A simple countermeasure would be to _include only the expected parameters in a legacy action_ (again a permitted list approach, as opposed to removing unexpected parameters). _And if you redirect to a URL, check it with a permitted list or a regular expression_. +If it is at the end of the URL it will hardly be noticed and redirects the user to the `attacker.com` host. As a general rule, passing user input directly into `redirect_to` is considered dangerous. A simple countermeasure would be to _include only the expected parameters in a legacy action_ (again a permitted list approach, as opposed to removing unexpected parameters). _And if you redirect to a URL, check it with a permitted list or a regular expression_. #### Self-contained XSS