-
Notifications
You must be signed in to change notification settings - Fork 266
Description
Query PR
Language
Ruby
CVE(s) ID list
- CVE-2022-23837
- In api.rb in Sidekiq, there is no limit on the number of days when requesting stats for the graph. This overloads the system, affecting the Web UI, and makes it unavailable to users.
- The commit of the fix is : sidekiq/sidekiq@7785ac1
- The vulnerable version of the database where the vulnerable path can be identified with this query is available at: https://raw.githubusercontent.com/Sim4n6/CodeQL_dbs/main/sidekiq_vuln4.zip
CWE
CWE-770: Allocation of Resources Without Limits or Throttling
Report
-
Application-level Denial of Service due to unconstrained use of a user controlled value (integer/float) in the allocation of a resource without limitation.
-
The source is a remote user controlled data, like
/?days=31
, through a vulnerable path this value without limitation reaches a ruby code that controls how many times a sync operation is repeated, like1.upto(days) do // something
. An exploit would be to issue/?days=9999999
to potentially cause an application-level denial of service remotely. -
I've studied the CVE-2022-23837 in sidekiq Denial of service. I have put focus on the fix commit sidekiq/sidekiq@7785ac1. I noticed the following:
- The source in sidekiq for this specific case is a remote user controlled parameter which goes encompassed thourgh a function called
params
too. So I extended the RemoteFlowSource::Range. - I noticed the essence of the problem comes from this line : https://github.com/sidekiq/sidekiq/blob/7785ac1399f1b28992adb56055f6acd88fd1d956/lib/sidekiq/api.rb#L182
dates = @start_date.downto(@start_date - @days_previous + 1).map { |date|
date.strftime("%Y-%m-%d")
}
The condition on the number of times the operation date.strftime()
is executed can be reached by a remote user data days_previous
.
-
I broadned the sink reach. The CVE fix considers limiting the pattern
A.downto(B)
but there are alsoA.upto(B)
andA.times()
. -
The for loop and unconditional loop appears to be of interest too, in a case like
for i in 1..days
. -
There was an additional flow step added for a case like the use of default value:
(days || 31).to_i
.
-
In case the incoming user data is limited, that is not a valid hit considered by the sanitizer
underAValue
. -
Other cases exist that could be considered as sanitizers
A.between?(1,100)
for instance.
Are you planning to discuss this vulnerability submission publicly? (Blog Post, social networks, etc).
- Yes
- No