Skip to content

Commit

Permalink
Use safe redirect paths in admin redirects
Browse files Browse the repository at this point in the history
This makes sure all redirects we do in the admin via do_redirect_to uses a safe redirect url.

(cherry picked from commit 7adefce)

# Conflicts:
#	app/controllers/alchemy/admin/base_controller.rb
#	app/controllers/alchemy/admin/pages_controller.rb
  • Loading branch information
tvdeyen committed Jan 7, 2025
1 parent 6b075b7 commit 0c65119
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 5 deletions.
13 changes: 11 additions & 2 deletions app/controllers/alchemy/admin/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ def is_safe_redirect_path?(path)
path.to_s.match? %r{^#{mount_path}admin/}
end

def relative_referer_path(referer = request.referer)
return unless referer

URI(referer).path
end

# Disable layout rendering for xhr requests.
def set_layout
request.xhr? ? false : "alchemy/admin"
Expand Down Expand Up @@ -122,13 +128,16 @@ def render_errors_or_redirect(object, redirect_url, flash_notice)

# Does redirects for html and js requests
#
# Makes sure that the redirect path is safe.
#
def do_redirect_to(url_or_path)
redirect_path = safe_redirect_path(url_or_path)
respond_to do |format|
format.js {
@redirect_url = url_or_path
@redirect_url = redirect_path
render :redirect
}
format.html { redirect_to url_or_path }
format.html { redirect_to redirect_path }

Check warning

Code scanning / CodeQL

URL redirection from remote source Medium

Untrusted URL redirection depends on a
user-provided value
.
Untrusted URL redirection depends on a
user-provided value
.
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/alchemy/admin/languages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def destroy
def switch
@language = set_alchemy_language(params[:language_id])
session[:alchemy_language_id] = @language.id
do_redirect_to request.referer || alchemy.admin_dashboard_path
do_redirect_to relative_referer_path || alchemy.admin_dashboard_path
end

private
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/alchemy/admin/pages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,17 @@ def unlock
format.js
format.html do
redirect_to(
params[:redirect_to].presence || admin_pages_path,
unlock_redirect_path,

Check warning

Code scanning / CodeQL

URL redirection from remote source Medium

Untrusted URL redirection depends on a
user-provided value
.
allow_other_host: true
)
end
end
end

def unlock_redirect_path
safe_redirect_path(fallback: admin_pages_path)
end

# Sets the page public and updates the published_at attribute that is used as cache_key
#
def publish
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/alchemy/admin/resources_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def destroy
flash[:error] = resource_instance_variable.errors.full_messages.join(", ")
end
flash_notice_for_resource_action
do_redirect_to resource_url_proxy.url_for(search_filter_params.merge(action: "index"))
do_redirect_to resource_url_proxy.url_for(search_filter_params.merge(action: "index", only_path: true))
end

def resource_handler
Expand Down

0 comments on commit 0c65119

Please sign in to comment.