ActionDispatch::Executor don't fully trust body#close

Under certain circumstances, the middleware isn't informed that the
response body has been fully closed which result in request state not
being fully reset before the next request.

[CVE-2022-23633]
This commit is contained in:
Jean Boussier 2022-02-11 13:09:30 +01:00 committed by Aaron Patterson
parent b50095f06c
commit 10c64a472f
No known key found for this signature in database
GPG Key ID: 953170BCB4FFAFC6
4 changed files with 49 additions and 24 deletions

@ -9,7 +9,7 @@ def initialize(app, executor)
end
def call(env)
state = @executor.run!
state = @executor.run!(reset: true)
begin
response = @app.call(env)
returned = response << ::Rack::BodyProxy.new(response.pop) { state.complete! }

@ -119,6 +119,27 @@ def test_callbacks_execute_in_shared_context
assert_not defined?(@in_shared_context) # it's not in the test itself
end
def test_body_abandonned
total = 0
ran = 0
completed = 0
executor.to_run { total += 1; ran += 1 }
executor.to_complete { total += 1; completed += 1}
stack = middleware(proc { [200, {}, "response"] })
requests_count = 5
requests_count.times do
stack.call({})
end
assert_equal (requests_count * 2) - 1, total
assert_equal requests_count, ran
assert_equal requests_count - 1, completed
end
private
def call_and_return_body(&block)
app = middleware(block || proc { [200, {}, "response"] })

@ -64,18 +64,21 @@ def self.register_hook(hook, outer: false)
# after the work has been performed.
#
# Where possible, prefer +wrap+.
def self.run!
if active?
Null
def self.run!(reset: false)
if reset
lost_instance = IsolatedExecutionState.delete(active_key)
lost_instance&.complete!
else
new.tap do |instance|
success = nil
begin
instance.run!
success = true
ensure
instance.complete! unless success
end
return Null if active?
end
new.tap do |instance|
success = nil
begin
instance.run!
success = true
ensure
instance.complete! unless success
end
end
end
@ -105,27 +108,20 @@ def self.perform # :nodoc:
end
end
class << self # :nodoc:
attr_accessor :active
end
def self.error_reporter
@error_reporter ||= ActiveSupport::ErrorReporter.new
end
def self.inherited(other) # :nodoc:
super
other.active = Concurrent::Hash.new
def self.active_key # :nodoc:
@active_key ||= :"active_execution_wrapper_#{object_id}"
end
self.active = Concurrent::Hash.new
def self.active? # :nodoc:
@active[IsolatedExecutionState.unique_id]
IsolatedExecutionState.key?(active_key)
end
def run! # :nodoc:
self.class.active[IsolatedExecutionState.unique_id] = true
IsolatedExecutionState[self.class.active_key] = self
run
end
@ -140,7 +136,7 @@ def run # :nodoc:
def complete!
complete
ensure
self.class.active.delete(IsolatedExecutionState.unique_id)
IsolatedExecutionState.delete(self.class.active_key)
end
def complete # :nodoc:

@ -42,6 +42,14 @@ def []=(key, value)
state[key] = value
end
def key?(key)
state.key?(key)
end
def delete(key)
state.delete(key)
end
def clear
state.clear
end