diff options
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 72 | ||||
| -rw-r--r-- | app/services/merge_requests/update_service.rb | 6 | ||||
| -rw-r--r-- | app/services/notification_service.rb | 2 | ||||
| -rw-r--r-- | app/workers/mail_scheduler/notification_service_worker.rb | 23 | ||||
| -rw-r--r-- | changelogs/unreleased/blackst0ne-bump-rails-cve-2018-16476.yml | 5 | ||||
| -rw-r--r-- | spec/workers/mail_scheduler/notification_service_worker_spec.rb | 17 |
7 files changed, 83 insertions, 44 deletions
@@ -1,6 +1,6 @@ source 'https://rubygems.org' -gem 'rails', '5.0.7' +gem 'rails', '5.0.7.1' gem 'rails-deprecated_sanitizer', '~> 1.0.3' # Improves copy-on-write performance for MRI diff --git a/Gemfile.lock b/Gemfile.lock index 08684cf7836..1f7f58d59e0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,41 +4,41 @@ GEM RedCloth (4.3.2) abstract_type (0.0.7) ace-rails-ap (4.1.2) - actioncable (5.0.7) - actionpack (= 5.0.7) + actioncable (5.0.7.1) + actionpack (= 5.0.7.1) nio4r (>= 1.2, < 3.0) websocket-driver (~> 0.6.1) - actionmailer (5.0.7) - actionpack (= 5.0.7) - actionview (= 5.0.7) - activejob (= 5.0.7) + actionmailer (5.0.7.1) + actionpack (= 5.0.7.1) + actionview (= 5.0.7.1) + activejob (= 5.0.7.1) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (5.0.7) - actionview (= 5.0.7) - activesupport (= 5.0.7) + actionpack (5.0.7.1) + actionview (= 5.0.7.1) + activesupport (= 5.0.7.1) rack (~> 2.0) rack-test (~> 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (5.0.7) - activesupport (= 5.0.7) + actionview (5.0.7.1) + activesupport (= 5.0.7.1) builder (~> 3.1) erubis (~> 2.7.0) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.0.3) - activejob (5.0.7) - activesupport (= 5.0.7) + activejob (5.0.7.1) + activesupport (= 5.0.7.1) globalid (>= 0.3.6) - activemodel (5.0.7) - activesupport (= 5.0.7) - activerecord (5.0.7) - activemodel (= 5.0.7) - activesupport (= 5.0.7) + activemodel (5.0.7.1) + activesupport (= 5.0.7.1) + activerecord (5.0.7.1) + activemodel (= 5.0.7.1) + activesupport (= 5.0.7.1) arel (~> 7.0) activerecord_sane_schema_dumper (1.0) rails (>= 5, < 6) - activesupport (5.0.7) + activesupport (5.0.7.1) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -381,7 +381,7 @@ GEM json (~> 1.8) multi_xml (>= 0.5.2) httpclient (2.8.3) - i18n (1.1.1) + i18n (1.2.0) concurrent-ruby (~> 1.0) icalendar (2.4.1) ice_nine (0.11.2) @@ -449,7 +449,7 @@ GEM loofah (2.2.3) crass (~> 1.0.2) nokogiri (>= 1.5.9) - mail (2.7.0) + mail (2.7.1) mini_mime (>= 0.1.1) mail_room (0.9.1) memoist (0.16.0) @@ -624,17 +624,17 @@ GEM rack rack-test (0.6.3) rack (>= 1.0) - rails (5.0.7) - actioncable (= 5.0.7) - actionmailer (= 5.0.7) - actionpack (= 5.0.7) - actionview (= 5.0.7) - activejob (= 5.0.7) - activemodel (= 5.0.7) - activerecord (= 5.0.7) - activesupport (= 5.0.7) + rails (5.0.7.1) + actioncable (= 5.0.7.1) + actionmailer (= 5.0.7.1) + actionpack (= 5.0.7.1) + actionview (= 5.0.7.1) + activejob (= 5.0.7.1) + activemodel (= 5.0.7.1) + activerecord (= 5.0.7.1) + activesupport (= 5.0.7.1) bundler (>= 1.3.0) - railties (= 5.0.7) + railties (= 5.0.7.1) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.2) actionpack (~> 5.x, >= 5.0.1) @@ -650,15 +650,15 @@ GEM rails-i18n (5.1.1) i18n (>= 0.7, < 2) railties (>= 5.0, < 6) - railties (5.0.7) - actionpack (= 5.0.7) - activesupport (= 5.0.7) + railties (5.0.7.1) + actionpack (= 5.0.7.1) + activesupport (= 5.0.7.1) method_source rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) rainbow (3.0.0) raindrops (0.18.0) - rake (12.3.1) + rake (12.3.2) rb-fsevent (0.10.2) rb-inotify (0.9.10) ffi (>= 0.5.0, < 2) @@ -1096,7 +1096,7 @@ DEPENDENCIES rack-cors (~> 1.0.0) rack-oauth2 (~> 1.2.1) rack-proxy (~> 0.6.0) - rails (= 5.0.7) + rails (= 5.0.7.1) rails-controller-testing rails-deprecated_sanitizer (~> 1.0.3) rails-i18n (~> 5.1) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 33d8299c8b6..86a04587f79 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -46,11 +46,13 @@ module MergeRequests end if merge_request.previous_changes.include?('assignee_id') + reassigned_merge_request_args = [merge_request, current_user] + old_assignee_id = merge_request.previous_changes['assignee_id'].first - old_assignee = User.find(old_assignee_id) if old_assignee_id + reassigned_merge_request_args << User.find(old_assignee_id) if old_assignee_id create_assignee_note(merge_request) - notification_service.async.reassigned_merge_request(merge_request, current_user, old_assignee) + notification_service.async.reassigned_merge_request(*reassigned_merge_request_args) todo_service.reassigned_merge_request(merge_request, current_user) end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index ff035fea216..e1cf327209b 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -188,7 +188,7 @@ class NotificationService # * merge_request assignee if their notification level is not Disabled # * users with custom level checked with "reassign merge request" # - def reassigned_merge_request(merge_request, current_user, previous_assignee) + def reassigned_merge_request(merge_request, current_user, previous_assignee = nil) recipients = NotificationRecipientService.build_recipients( merge_request, current_user, diff --git a/app/workers/mail_scheduler/notification_service_worker.rb b/app/workers/mail_scheduler/notification_service_worker.rb index 4726e416182..c8ccaf0c487 100644 --- a/app/workers/mail_scheduler/notification_service_worker.rb +++ b/app/workers/mail_scheduler/notification_service_worker.rb @@ -8,14 +8,35 @@ module MailScheduler include MailSchedulerQueue def perform(meth, *args) - deserialized_args = ActiveJob::Arguments.deserialize(args) + check_arguments!(args) + deserialized_args = ActiveJob::Arguments.deserialize(args) notification_service.public_send(meth, *deserialized_args) # rubocop:disable GitlabSecurity/PublicSend rescue ActiveJob::DeserializationError + # No-op. + # This exception gets raised when an argument + # is correct (deserializeable), but it still cannot be deserialized. + # This can happen when an object has been deleted after + # rails passes this job to sidekiq, but before + # sidekiq gets it for execution. + # In this case just do nothing. end def self.perform_async(*args) super(*ActiveJob::Arguments.serialize(args)) end + + private + + # If an argument is in the ActiveJob::Arguments::TYPE_WHITELIST list, + # it means the argument cannot be deserialized. + # Which means there's something wrong with our code. + def check_arguments!(args) + args.each do |arg| + if arg.class.in?(ActiveJob::Arguments::TYPE_WHITELIST) + raise(ArgumentError, "Argument `#{arg}` cannot be deserialized because of its type") + end + end + end end end diff --git a/changelogs/unreleased/blackst0ne-bump-rails-cve-2018-16476.yml b/changelogs/unreleased/blackst0ne-bump-rails-cve-2018-16476.yml new file mode 100644 index 00000000000..dfa94c69ce0 --- /dev/null +++ b/changelogs/unreleased/blackst0ne-bump-rails-cve-2018-16476.yml @@ -0,0 +1,5 @@ +--- +title: Bump Ruby on Rails to 5.0.7.1 +merge_request: 23396 +author: "@blackst0ne" +type: security diff --git a/spec/workers/mail_scheduler/notification_service_worker_spec.rb b/spec/workers/mail_scheduler/notification_service_worker_spec.rb index f725c8763a0..1033557ee88 100644 --- a/spec/workers/mail_scheduler/notification_service_worker_spec.rb +++ b/spec/workers/mail_scheduler/notification_service_worker_spec.rb @@ -17,10 +17,21 @@ describe MailScheduler::NotificationServiceWorker do end context 'when the arguments cannot be deserialized' do - it 'does nothing' do - expect(worker.notification_service).not_to receive(method) + context 'when the arguments are not deserializeable' do + it 'raises exception' do + expect(worker.notification_service).not_to receive(method) + expect { worker.perform(method, key.to_global_id.to_s.succ) }.to raise_exception(ArgumentError) + end + end + + context 'when the arguments are deserializeable' do + it 'does nothing' do + serialized_arguments = *serialize(key) + key.destroy! - worker.perform(method, key.to_global_id.to_s.succ) + expect(worker.notification_service).not_to receive(method) + expect { worker.perform(method, serialized_arguments) }.not_to raise_exception + end end end |