Skip to content

Verify method in MailScheduler::NotificationServiceWorker

What does this MR do and why?

This worker is very generic, and it can call any method on NotificationService (including those defined in EE and JH). This change ensures that the method is defined only on those classes, not on a superclass like Object.

This has a feature flag, disabled by default, to make failing that check raise ArgumentError.

A Sidekiq job class that takes a method name as an argument and calls that method (with public_send on an object) is a security risk. It's not typically directly exploitable, but it can be used to escalate from Redis injection to arbitrary code execution (see #371098 (closed) for an example).

We should remove public_send from Sidekiq workers where possible, and where not possible, validate the method name we're passing carefully. I found these workers that disable the Security/PublicSend cop:

Split from !103053 (closed). For #371470 (closed).

How to set up and validate locally

Mostly just specs.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports

Loading