From cdc2bc43d4b40d6bb5d3ab9ecbff509634360db6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Fri, 24 Aug 2018 16:52:23 +0000 Subject: [PATCH] [master] Missing CSRF in System Hooks resend action --- app/views/admin/hook_logs/show.html.haml | 3 +-- app/views/projects/hook_logs/show.html.haml | 2 +- .../security-fj-missing-csrf-system-hooks-resend.yml | 5 +++++ config/routes/admin.rb | 2 +- config/routes/project.rb | 2 +- spec/routing/admin_routing_spec.rb | 4 ++-- spec/routing/project_routing_spec.rb | 6 +++--- 7 files changed, 14 insertions(+), 10 deletions(-) create mode 100644 changelogs/unreleased/security-fj-missing-csrf-system-hooks-resend.yml diff --git a/app/views/admin/hook_logs/show.html.haml b/app/views/admin/hook_logs/show.html.haml index 2eb3ac85722..86729dbe7bc 100644 --- a/app/views/admin/hook_logs/show.html.haml +++ b/app/views/admin/hook_logs/show.html.haml @@ -4,7 +4,6 @@ %hr -= link_to 'Resend Request', retry_admin_hook_hook_log_path(@hook, @hook_log), class: "btn btn-default float-right prepend-left-10" += link_to 'Resend Request', retry_admin_hook_hook_log_path(@hook, @hook_log), method: :post, class: "btn btn-default float-right prepend-left-10" = render partial: 'shared/hook_logs/content', locals: { hook_log: @hook_log } - diff --git a/app/views/projects/hook_logs/show.html.haml b/app/views/projects/hook_logs/show.html.haml index e51efa85df0..bd8ca5e7d70 100644 --- a/app/views/projects/hook_logs/show.html.haml +++ b/app/views/projects/hook_logs/show.html.haml @@ -4,6 +4,6 @@ Request details .col-lg-9 - = link_to 'Resend Request', retry_project_hook_hook_log_path(@project, @hook, @hook_log), class: "btn btn-default float-right prepend-left-10" + = link_to 'Resend Request', retry_project_hook_hook_log_path(@project, @hook, @hook_log), method: :post, class: "btn btn-default float-right prepend-left-10" = render partial: 'shared/hook_logs/content', locals: { hook_log: @hook_log } diff --git a/changelogs/unreleased/security-fj-missing-csrf-system-hooks-resend.yml b/changelogs/unreleased/security-fj-missing-csrf-system-hooks-resend.yml new file mode 100644 index 00000000000..018acb9c5af --- /dev/null +++ b/changelogs/unreleased/security-fj-missing-csrf-system-hooks-resend.yml @@ -0,0 +1,5 @@ +--- +title: Adding CSRF protection to Hooks resend action +merge_request: +author: +type: security diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 7ee960970f8..fa1f79a90be 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -59,7 +59,7 @@ namespace :admin do resources :hook_logs, only: [:show] do member do - get :retry + post :retry end end end diff --git a/config/routes/project.rb b/config/routes/project.rb index 0220e88c819..34f49546983 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -307,7 +307,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do resources :hook_logs, only: [:show] do member do - get :retry + post :retry end end end diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb index 98df5f787f7..77baaef7afd 100644 --- a/spec/routing/admin_routing_spec.rb +++ b/spec/routing/admin_routing_spec.rb @@ -103,11 +103,11 @@ describe Admin::HooksController, "routing" do end end -# admin_hook_hook_log_retry GET /admin/hooks/:hook_id/hook_logs/:id/retry(.:format) admin/hook_logs#retry +# admin_hook_hook_log_retry POST /admin/hooks/:hook_id/hook_logs/:id/retry(.:format) admin/hook_logs#retry # admin_hook_hook_log GET /admin/hooks/:hook_id/hook_logs/:id(.:format) admin/hook_logs#show describe Admin::HookLogsController, 'routing' do it 'to #retry' do - expect(get('/admin/hooks/1/hook_logs/1/retry')).to route_to('admin/hook_logs#retry', hook_id: '1', id: '1') + expect(post('/admin/hooks/1/hook_logs/1/retry')).to route_to('admin/hook_logs#retry', hook_id: '1', id: '1') end it 'to #show' do diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 70a7707826e..5abc6d81958 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -381,7 +381,7 @@ describe 'project routing' do end end - # test_project_hook GET /:project_id/hooks/:id/test(.:format) hooks#test + # test_project_hook POST /:project_id/hooks/:id/test(.:format) hooks#test # project_hooks GET /:project_id/hooks(.:format) hooks#index # POST /:project_id/hooks(.:format) hooks#create # edit_project_hook GET /:project_id/hooks/:id/edit(.:format) hooks#edit @@ -398,11 +398,11 @@ describe 'project routing' do end end - # retry_namespace_project_hook_hook_log GET /:project_id/hooks/:hook_id/hook_logs/:id/retry(.:format) projects/hook_logs#retry + # retry_namespace_project_hook_hook_log POST /:project_id/hooks/:hook_id/hook_logs/:id/retry(.:format) projects/hook_logs#retry # namespace_project_hook_hook_log GET /:project_id/hooks/:hook_id/hook_logs/:id(.:format) projects/hook_logs#show describe Projects::HookLogsController, 'routing' do it 'to #retry' do - expect(get('/gitlab/gitlabhq/hooks/1/hook_logs/1/retry')).to route_to('projects/hook_logs#retry', namespace_id: 'gitlab', project_id: 'gitlabhq', hook_id: '1', id: '1') + expect(post('/gitlab/gitlabhq/hooks/1/hook_logs/1/retry')).to route_to('projects/hook_logs#retry', namespace_id: 'gitlab', project_id: 'gitlabhq', hook_id: '1', id: '1') end it 'to #show' do