diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 24651dd392cf0cbe6897d2e1d658286f89d02370..0fdd4d2cb47fac14ec21d036f3aec9fc4e05da69 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -5,6 +5,7 @@ class ApplicationController < ActionController::Base include Gitlab::GonHelper include GitlabRoutingHelper include PageLayoutHelper + include SafeParamsHelper include SentryHelper include WorkhorseHelper include EnforcesTwoFactorAuthentication diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index e89eaf7eddaba7d016e96dfda8b9e59ce19624cb..f9e8fe624e8572c8046b519c61e4febcf84c4adb 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -86,7 +86,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController out_of_range = todos.current_page > total_pages if out_of_range - redirect_to url_for(params.merge(page: total_pages, only_path: true)) + redirect_to url_for(safe_params.merge(page: total_pages, only_path: true)) end out_of_range diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 5ac4b8710e28767416bbb2be78d9e23641e92ab6..79fa581835906af9c3e58ee179404a08c9fc1ac3 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -189,6 +189,6 @@ class GroupsController < Groups::ApplicationController params[:id] = group.to_param - url_for(params) + url_for(safe_params) end end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 37f14230196accda7d9d9d7e272bf9bfdc002113..a93b116c6fe1f9fcace7e5ec65b62f39d5aec27d 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -404,7 +404,7 @@ class ProjectsController < Projects::ApplicationController params[:namespace_id] = project.namespace.to_param params[:id] = project.to_param - url_for(params) + url_for(safe_params) end def project_export_enabled diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 956df4a0a1687e44bfb714afa97a8992031d07ec..31f47a7aa7c21a3bc12b3d0b1d7a410d24919cc8 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -146,6 +146,6 @@ class UsersController < ApplicationController end def build_canonical_path(user) - url_for(params.merge(username: user.to_param)) + url_for(safe_params.merge(username: user.to_param)) end end diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 866b8773db6b992540cb7d5c33e52a5ebdaefe2d..fef29789832d6d09ee0464ce868e3cdbe4f28991 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -259,7 +259,7 @@ module BlobHelper options = [] if error == :collapsed - options << link_to('load it anyway', url_for(params.merge(viewer: viewer.type, expanded: true, format: nil))) + options << link_to('load it anyway', url_for(safe_params.merge(viewer: viewer.type, expanded: true, format: nil))) end # If the error is `:server_side_but_stored_externally`, the simple viewer will show the same error, diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index b5ca39711bc3076830542647dfc3d5fbeb9950dc..1bb82fd81503831b212b2e5549f5ebaaa70695f0 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -180,7 +180,7 @@ module DiffHelper private def diff_btn(title, name, selected) - params_copy = params.dup + params_copy = safe_params.dup params_copy[:view] = name # Always use HTML to handle case where JSON diff rendered this button diff --git a/app/helpers/safe_params_helper.rb b/app/helpers/safe_params_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..b568e8810cce6973be7c5c6ecebc272b484bfbf8 --- /dev/null +++ b/app/helpers/safe_params_helper.rb @@ -0,0 +1,11 @@ +module SafeParamsHelper + # Rails 5.0 requires to permit `params` if they're used in url helpers. + # Use this helper when generating links with `params.merge(...)` + def safe_params + if params.respond_to?(:permit!) + params.except(:host, :port, :protocol).permit! + else + params + end + end +end diff --git a/app/views/dashboard/issues.atom.builder b/app/views/dashboard/issues.atom.builder index 70ec6bc62576fc6fc39a4e7dc208fb302aacb613..d7b6fb9a4a144c7082eaaa3f4a3601191cde4746 100644 --- a/app/views/dashboard/issues.atom.builder +++ b/app/views/dashboard/issues.atom.builder @@ -1,5 +1,5 @@ xml.title "#{current_user.name} issues" -xml.link href: url_for(params), rel: "self", type: "application/atom+xml" +xml.link href: url_for(safe_params), rel: "self", type: "application/atom+xml" xml.link href: issues_dashboard_url, rel: "alternate", type: "text/html" xml.id issues_dashboard_url xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? diff --git a/app/views/dashboard/issues.html.haml b/app/views/dashboard/issues.html.haml index bb472b4c90030e34060efa43f6264bd80a24f300..4bf04dadf01a016512a5862342ce768b946166bb 100644 --- a/app/views/dashboard/issues.html.haml +++ b/app/views/dashboard/issues.html.haml @@ -2,12 +2,12 @@ - page_title _("Issues") - @breadcrumb_link = issues_dashboard_path(assignee_id: current_user.id) = content_for :meta_tags do - = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{current_user.name} issues") + = auto_discovery_link_tag(:atom, safe_params.merge(rss_url_options).to_h, title: "#{current_user.name} issues") .top-area = render 'shared/issuable/nav', type: :issues, display_count: !@no_filters_set .nav-controls - = link_to params.merge(rss_url_options), class: 'btn has-tooltip', data: { container: 'body' }, title: 'Subscribe' do + = link_to safe_params.merge(rss_url_options), class: 'btn has-tooltip', data: { container: 'body' }, title: 'Subscribe' do = icon('rss') = render 'shared/new_project_item_select', path: 'issues/new', label: "New issue", with_feature_enabled: 'issues', type: :issues diff --git a/app/views/groups/issues.atom.builder b/app/views/groups/issues.atom.builder index a239ea8caf009fe7dd82b032697636b07852ad76..2a385b661e50f910067b9ee161198a87f8286891 100644 --- a/app/views/groups/issues.atom.builder +++ b/app/views/groups/issues.atom.builder @@ -1,5 +1,5 @@ xml.title "#{@group.name} issues" -xml.link href: url_for(params), rel: "self", type: "application/atom+xml" +xml.link href: url_for(safe_params), rel: "self", type: "application/atom+xml" xml.link href: issues_group_url, rel: "alternate", type: "text/html" xml.id issues_group_url xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? diff --git a/app/views/groups/issues.html.haml b/app/views/groups/issues.html.haml index 36df03302e8a7633461040f9d169589793a11c7d..bbfbea4ac7a2b9281ece3e7ab0c3c28ae8d18dfc 100644 --- a/app/views/groups/issues.html.haml +++ b/app/views/groups/issues.html.haml @@ -1,6 +1,6 @@ - page_title "Issues" = content_for :meta_tags do - = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@group.name} issues") + = auto_discovery_link_tag(:atom, safe_params.merge(rss_url_options).to_h, title: "#{@group.name} issues") - if group_issues_count(state: 'all').zero? = render 'shared/empty_states/issues', project_select_button: true diff --git a/app/views/projects/blob/_viewer.html.haml b/app/views/projects/blob/_viewer.html.haml index 9c760c81527fc7cd7aa9bc35f9f06f2017b7cd26..b9663bbba1525ff366e12f466f64ae0563c79956 100644 --- a/app/views/projects/blob/_viewer.html.haml +++ b/app/views/projects/blob/_viewer.html.haml @@ -4,7 +4,7 @@ - load_async = local_assigns.fetch(:load_async, viewer.load_async? && render_error.nil?) - external_embed = local_assigns.fetch(:external_embed, false) -- viewer_url = local_assigns.fetch(:viewer_url) { url_for(params.merge(viewer: viewer.type, format: :json)) } if load_async +- viewer_url = local_assigns.fetch(:viewer_url) { url_for(safe_params.merge(viewer: viewer.type, format: :json)) } if load_async .blob-viewer{ data: { type: viewer.type, rich_type: rich_type, url: viewer_url }, class: ('hidden' if hidden) } - if render_error = render 'projects/blob/render_error', viewer: viewer diff --git a/app/views/projects/diffs/_collapsed.html.haml b/app/views/projects/diffs/_collapsed.html.haml index 8772bd4705f2815a18f096ea16e8034a62e8ab7e..5762f4d86d7ee532c8242b560dfb7f3ca8cd0766 100644 --- a/app/views/projects/diffs/_collapsed.html.haml +++ b/app/views/projects/diffs/_collapsed.html.haml @@ -1,5 +1,5 @@ - diff_file = viewer.diff_file -- url = url_for(params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier)) +- url = url_for(safe_params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier)) .nothing-here-block.diff-collapsed{ data: { diff_for_path: url } } This diff is collapsed. %a.click-to-expand Click to expand it. diff --git a/app/views/projects/issues/_nav_btns.html.haml b/app/views/projects/issues/_nav_btns.html.haml index dd1a836fa2030732c3979a4cd047c68bb3b73f0d..297b928f020ae2e3289c205b705d4b422545912f 100644 --- a/app/views/projects/issues/_nav_btns.html.haml +++ b/app/views/projects/issues/_nav_btns.html.haml @@ -1,4 +1,4 @@ -= link_to params.merge(rss_url_options), class: 'btn btn-default append-right-10 has-tooltip', title: 'Subscribe' do += link_to safe_params.merge(rss_url_options), class: 'btn btn-default append-right-10 has-tooltip', title: 'Subscribe' do = icon('rss') - if @can_bulk_update = button_tag "Edit issues", class: "btn btn-default append-right-10 js-bulk-update-toggle" diff --git a/app/views/projects/issues/index.atom.builder b/app/views/projects/issues/index.atom.builder index 4029926f373dcb286cc207dfcc1f85e7c3f1be90..6330245954e743e5d2a8b330610c53005ec7e410 100644 --- a/app/views/projects/issues/index.atom.builder +++ b/app/views/projects/issues/index.atom.builder @@ -1,5 +1,5 @@ xml.title "#{@project.name} issues" -xml.link href: url_for(params), rel: "self", type: "application/atom+xml" +xml.link href: url_for(safe_params), rel: "self", type: "application/atom+xml" xml.link href: project_issues_url(@project), rel: "alternate", type: "text/html" xml.id project_issues_url(@project) xml.updated @issues.first.updated_at.xmlschema if @issues.reorder(nil).any? diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml index c427a9eedc266ff0bb7fe2b8893b5d8c999d588d..1e7737aeb976609c796be8fa73963c1064517f48 100644 --- a/app/views/projects/issues/index.html.haml +++ b/app/views/projects/issues/index.html.haml @@ -5,7 +5,7 @@ - new_issue_email = @project.new_issuable_address(current_user, 'issue') = content_for :meta_tags do - = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@project.name} issues") + = auto_discovery_link_tag(:atom, safe_params.merge(rss_url_options).to_h, title: "#{@project.name} issues") - if project_issues(@project).exists? %div{ class: (container_class) } diff --git a/app/views/projects/merge_requests/creations/_new_submit.html.haml b/app/views/projects/merge_requests/creations/_new_submit.html.haml index 376ac377562cb392dd5f45e98b7342f91cfdd003..68780cedeb1963ca9b0607f4678bec9328344d32 100644 --- a/app/views/projects/merge_requests/creations/_new_submit.html.haml +++ b/app/views/projects/merge_requests/creations/_new_submit.html.haml @@ -26,16 +26,16 @@ - else %ul.merge-request-tabs.nav-links.no-top.no-bottom %li.commits-tab.active - = link_to url_for(params), data: {target: 'div#commits', action: 'new', toggle: 'tab'} do + = link_to url_for(safe_params), data: {target: 'div#commits', action: 'new', toggle: 'tab'} do Commits %span.badge= @commits.size - if @pipelines.any? %li.builds-tab - = link_to url_for(params.merge(action: 'pipelines')), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do + = link_to url_for(safe_params.merge(action: 'pipelines')), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do Pipelines %span.badge= @pipelines.size %li.diffs-tab - = link_to url_for(params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tab'} do + = link_to url_for(safe_params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tab'} do Changes %span.badge= @merge_request.diff_size @@ -46,7 +46,7 @@ -# This tab is always loaded via AJAX - if @pipelines.any? #pipelines.pipelines.tab-pane - = render 'projects/merge_requests/pipelines', endpoint: url_for(params.merge(action: 'pipelines', format: :json)), disable_initialization: true + = render 'projects/merge_requests/pipelines', endpoint: url_for(safe_params.merge(action: 'pipelines', format: :json)), disable_initialization: true .mr-loading-status = spinner