Skip to content

Commit 7785ac1

Browse files
committed
Validate days parameter to avoid possible DoS in Web UI
Thank you to Sergey Shpakov of http://tutum.space for reporting.
1 parent 0a4de94 commit 7785ac1

File tree

4 files changed

+17
-2
lines changed

4 files changed

+17
-2
lines changed

lib/sidekiq/api.rb

+2
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ def lengths
161161

162162
class History
163163
def initialize(days_previous, start_date = nil)
164+
# we only store five years of data in Redis
165+
raise ArgumentError if days_previous < 1 || days_previous > (5 * 365)
164166
@days_previous = days_previous
165167
@start_date = start_date || Time.now.utc.to_date
166168
end

lib/sidekiq/web/application.rb

+4-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ def self.set(key, val)
5050

5151
get "/" do
5252
@redis_info = redis_info.select { |k, v| REDIS_KEYS.include? k }
53-
stats_history = Sidekiq::Stats::History.new((params["days"] || 30).to_i)
53+
days = (params["days"] || 30).to_i
54+
return halt(401) if days < 1 || days > 180
55+
56+
stats_history = Sidekiq::Stats::History.new(days)
5457
@processed_history = stats_history.processed
5558
@failed_history = stats_history.failed
5659

test/test_api.rb

+9
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,15 @@
156156
Time::DATE_FORMATS[:default] = @before
157157
end
158158

159+
describe "history" do
160+
it "does not allow invalid input" do
161+
assert_raises(ArgumentError) { Sidekiq::Stats::History.new(-1) }
162+
assert_raises(ArgumentError) { Sidekiq::Stats::History.new(0) }
163+
assert_raises(ArgumentError) { Sidekiq::Stats::History.new(2000) }
164+
assert Sidekiq::Stats::History.new(200)
165+
end
166+
end
167+
159168
describe "processed" do
160169
it 'retrieves hash of dates' do
161170
Sidekiq.redis do |c|

test/test_web.rb

+2-1
Original file line numberDiff line numberDiff line change
@@ -748,8 +748,9 @@ def app
748748
basic_authorize 'a', 'b'
749749

750750
get '/'
751-
752751
assert_equal 200, last_response.status
752+
get '/?days=1000000'
753+
assert_equal 401, last_response.status
753754
end
754755
end
755756

0 commit comments

Comments
 (0)