From 4372c3897b7f9fbafe1d9fd1c281ae7d86f5a2a1 Mon Sep 17 00:00:00 2001
From: Mauro Mottini <mottinimauro@gmail.com>
Date: Mon, 30 Aug 2021 21:58:56 -0300
Subject: [PATCH] Fix open redirect vulnerability

An open redirect can be possible when users are able to set the value of
session[:return_to]. If the value used for return_to contains multiple
leading slashes (/////example.com) the user ends up being redirected the
external domain that comes after the slashes (http://example.com).

To fix this issue, extra sanitization was added when processing the
return_to url, removing multiple leading slashes to avoid the open
redirect.

Co-authored-by: Kirill Efimov <kirill89@gmail.com>
---
 lib/clearance/authorization.rb               |  8 +++++++-
 spec/controllers/sessions_controller_spec.rb | 13 +++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/clearance/authorization.rb b/lib/clearance/authorization.rb
index 4fb574e39..0a36fb3dd 100644
--- a/lib/clearance/authorization.rb
+++ b/lib/clearance/authorization.rb
@@ -86,10 +86,16 @@ def redirect_back_or(default)
     def return_to
       if return_to_url
         uri = URI.parse(return_to_url)
-        "#{uri.path}?#{uri.query}".chomp("?") + "##{uri.fragment}".chomp("#")
+        path = path_without_leading_slashes(uri)
+        "#{path}?#{uri.query}".chomp("?") + "##{uri.fragment}".chomp("#")
       end
     end
 
+    # @api private
+    def path_without_leading_slashes(uri)
+      uri.path.sub(/\A\/+/, "/")
+    end
+
     # @api private
     def return_to_url
       session[:return_to]
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 17575d8c4..f067f9b8b 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -58,6 +58,19 @@
     end
 
     context "with good credentials and a session return url" do
+      it "redirects to the return URL removing leading slashes" do
+        user = create(:user)
+        url = "/url_in_the_session?foo=bar#baz"
+        return_url = "//////#{url}"
+        request.session[:return_to] = return_url
+
+        post :create, params: {
+          session: { email: user.email, password: user.password },
+        }
+
+        should redirect_to(url)
+      end
+
       it "redirects to the return URL maintaining query and fragment" do
         user = create(:user)
         return_url = "/url_in_the_session?foo=bar#baz"