From 37f2d5925fbaefa34a3a23d0c25237fbdf237307 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 7 Jan 2022 20:44:37 -0600 Subject: [PATCH] sessions: fix open redirect in login page. Fix an open redirect exploit where if you went to , then after you logged in you would be redirected to https://fakebooru.com. This was actually fixed by the upgrade to Rails 7.0. `redirect_to` now raises an `UnsafeRedirectError` on redirect to an offsite URL. Before we tried to prevent offsite redirects by checking that the URL started with a slash, but this was insufficient - it allowed protocol-relative URLs like `//fakebooru.com`. Add a test case for protocol-relative URLs and return a 403 error on an offsite redirect. --- app/controllers/application_controller.rb | 2 +- app/controllers/sessions_controller.rb | 2 +- test/functional/sessions_controller_test.rb | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4b2775887..bfd494a02 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -115,7 +115,7 @@ class ApplicationController < ActionController::Base render_error_page(400, exception) when SessionLoader::AuthenticationFailure render_error_page(401, exception, template: "sessions/new") - when ActionController::InvalidAuthenticityToken, ActionController::UnpermittedParameters, ActionController::InvalidCrossOriginRequest + when ActionController::InvalidAuthenticityToken, ActionController::UnpermittedParameters, ActionController::InvalidCrossOriginRequest, ActionController::Redirecting::UnsafeRedirectError render_error_page(403, exception) when ActiveSupport::MessageVerifier::InvalidSignature, # raised by `find_signed!` User::PrivilegeError, diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 7cc67b7a9..be32d8303 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -16,9 +16,9 @@ class SessionsController < ApplicationController def create name, password, url = params.fetch(:session, params).slice(:name, :password, :url).values user = SessionLoader.new(request).login(name, password) + url ||= posts_path if user - url = posts_path unless url&.start_with?("/") respond_with(user, location: url) else flash.now[:notice] = "Password was incorrect" diff --git a/test/functional/sessions_controller_test.rb b/test/functional/sessions_controller_test.rb index eb9ab6fb6..3cac418cd 100644 --- a/test/functional/sessions_controller_test.rb +++ b/test/functional/sessions_controller_test.rb @@ -43,6 +43,11 @@ class SessionsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to tags_path end + should "not allow redirects to protocol-relative URLs" do + post session_path, params: { name: @user.name, password: "password", url: "//example.com" } + assert_response 403 + end + should "not allow IP banned users to login" do @ip_ban = create(:ip_ban, category: :full, ip_addr: "1.2.3.4") post session_path, params: { name: @user.name, password: "password" }, headers: { REMOTE_ADDR: "1.2.3.4" }