[prev in list] [next in list] [prev in thread] [next in thread]
List: oss-security
Subject: [oss-security] [CVE-2022-23633] Possible exposure of information vulnerability in Action Pack
From: Aaron Patterson <aaron.patterson () gmail ! com>
Date: 2022-02-11 20:39:10
Message-ID: CALBaBG95cw-_ZPCi+pppp_uhvX1ydSf=FP+sxfG-j4GDvieBFQ () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
## Impact
Under certain circumstances response bodies will not be closed, for example
a bug in a webserver[1] or a bug in a Rack middleware. In the event a
response is not notified of a close, ActionDispatch::Executor will not know
to reset thread local state for the next request. This can lead to data
being leaked to subsequent requests, especially when interacting with
ActiveSupport::CurrentAttributes.
Upgrading to the FIXED versions of Rails will ensure mitigation if this
issue even in the context of a buggy webserver or middleware implementation.
## Patches
To aid users who aren't able to upgrade immediately we have provided
patches for
the two supported release series. They are in git-am format and consist of a
single changeset.
* 5.2-information-leak.patch
* 6.0-information-leak.patch
* 6.1-information-leak.patch
* 7.0-information-leak.patch
## Workarounds
Upgrading is highly recommended, but to work around this problem the
following middleware can be used:
```
class GuardedExecutor < ActionDispatch::Executor
def call(env)
ensure_completed!
super
end
private
def ensure_completed!
@executor.new.complete! if @executor.active?
end
end
# Ensure the guard is inserted before ActionDispatch::Executor
Rails.application.configure do
config.middleware.swap ActionDispatch::Executor, GuardedExecutor, executor
end
```
## Credits
Thanks to Jean Boussier for fixing this!
1. https://github.com/puma/puma/pull/2812
[Attachment #5 (text/html)]
<div dir="ltr">## Impact<br><br>Under certain circumstances response bodies will not be closed, \
for example a bug in a webserver[1] or a bug in a Rack middleware. In the event a response is \
not notified of a close, ActionDispatch::Executor will not know to reset thread local state for \
the next request. This can lead to data being leaked to subsequent requests, especially when \
interacting with ActiveSupport::CurrentAttributes.<br><br>Upgrading to the FIXED versions of \
Rails will ensure mitigation if this issue even in the context of a buggy webserver or \
middleware implementation.<br><br>## Patches<br><br>To aid users who aren't able to upgrade \
immediately we have provided patches for<br>the two supported release series. They are in \
git-am format and consist of a<br>single changeset.<br><br>* 5.2-information-leak.patch<br>* \
6.0-information-leak.patch<br>* 6.1-information-leak.patch<br>* \
7.0-information-leak.patch<br><br>## Workarounds<br><br>Upgrading is highly recommended, but to \
work around this problem the following middleware can be used:<br><br>```<br>class \
GuardedExecutor < ActionDispatch::Executor<br> def call(env)<br> \
ensure_completed!<br> super<br> end<br><br> private<br><br> def \
ensure_completed!<br> @executor.new.complete! if @executor.active?<br> \
end<br>end<br><br># Ensure the guard is inserted before \
ActionDispatch::Executor<br>Rails.application.configure do<br> config.middleware.swap \
ActionDispatch::Executor, GuardedExecutor, executor<br>end<br>```<br><br>## \
Credits<br><br>Thanks to Jean Boussier for fixing this!<br><br>1. <a \
href="https://github.com/puma/puma/pull/2812">https://github.com/puma/puma/pull/2812</a><br><br></div>
--000000000000c2856705d7c40f82--
["6.1-information-leak.patch" (application/octet-stream)]
From 7d27d53f6aa54687f37ae6b0575e736aff999b97 Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Fri, 11 Feb 2022 13:09:30 +0100
Subject: [PATCH] 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]
---
.../action_dispatch/middleware/executor.rb | 2 +-
actionpack/test/dispatch/executor_test.rb | 21 ++++++++++++++
.../lib/active_support/execution_wrapper.rb | 29 ++++++++++---------
activesupport/lib/active_support/reloader.rb | 2 +-
4 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/actionpack/lib/action_dispatch/middleware/executor.rb \
b/actionpack/lib/action_dispatch/middleware/executor.rb index 129b18d3d9..a32f916260 100644
--- a/actionpack/lib/action_dispatch/middleware/executor.rb
+++ b/actionpack/lib/action_dispatch/middleware/executor.rb
@@ -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! }
diff --git a/actionpack/test/dispatch/executor_test.rb \
b/actionpack/test/dispatch/executor_test.rb index 5b8be39b6d..d0bf574009 100644
--- a/actionpack/test/dispatch/executor_test.rb
+++ b/actionpack/test/dispatch/executor_test.rb
@@ -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"] })
diff --git a/activesupport/lib/active_support/execution_wrapper.rb \
b/activesupport/lib/active_support/execution_wrapper.rb index ca810db584..07c4f435db 100644
--- a/activesupport/lib/active_support/execution_wrapper.rb
+++ b/activesupport/lib/active_support/execution_wrapper.rb
@@ -63,18 +63,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 = active.delete(Thread.current)
+ 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
@@ -103,11 +106,11 @@ def self.inherited(other) # :nodoc:
self.active = Concurrent::Hash.new
def self.active? # :nodoc:
- @active[Thread.current]
+ @active.key?(Thread.current)
end
def run! # :nodoc:
- self.class.active[Thread.current] = true
+ self.class.active[Thread.current] = self
run_callbacks(:run)
end
diff --git a/activesupport/lib/active_support/reloader.rb \
b/activesupport/lib/active_support/reloader.rb index 2f81cd4f80..e751866a27 100644
--- a/activesupport/lib/active_support/reloader.rb
+++ b/activesupport/lib/active_support/reloader.rb
@@ -58,7 +58,7 @@ def self.reload!
prepare!
end
- def self.run! # :nodoc:
+ def self.run!(reset: false) # :nodoc:
if check!
super
else
--
2.35.0
["6.0-information-leak.patch" (application/octet-stream)]
From f53020bdf80d75b5e9451ee3b286905ac0dc24e2 Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Fri, 11 Feb 2022 13:09:30 +0100
Subject: [PATCH] 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]
---
.../action_dispatch/middleware/executor.rb | 2 +-
actionpack/test/dispatch/executor_test.rb | 21 ++++++++++++++
.../lib/active_support/execution_wrapper.rb | 29 ++++++++++---------
activesupport/lib/active_support/reloader.rb | 2 +-
4 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/actionpack/lib/action_dispatch/middleware/executor.rb \
b/actionpack/lib/action_dispatch/middleware/executor.rb index 129b18d3d9..a32f916260 100644
--- a/actionpack/lib/action_dispatch/middleware/executor.rb
+++ b/actionpack/lib/action_dispatch/middleware/executor.rb
@@ -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! }
diff --git a/actionpack/test/dispatch/executor_test.rb \
b/actionpack/test/dispatch/executor_test.rb index 5b8be39b6d..d0bf574009 100644
--- a/actionpack/test/dispatch/executor_test.rb
+++ b/actionpack/test/dispatch/executor_test.rb
@@ -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"] })
diff --git a/activesupport/lib/active_support/execution_wrapper.rb \
b/activesupport/lib/active_support/execution_wrapper.rb index ca810db584..07c4f435db 100644
--- a/activesupport/lib/active_support/execution_wrapper.rb
+++ b/activesupport/lib/active_support/execution_wrapper.rb
@@ -63,18 +63,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 = active.delete(Thread.current)
+ 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
@@ -103,11 +106,11 @@ def self.inherited(other) # :nodoc:
self.active = Concurrent::Hash.new
def self.active? # :nodoc:
- @active[Thread.current]
+ @active.key?(Thread.current)
end
def run! # :nodoc:
- self.class.active[Thread.current] = true
+ self.class.active[Thread.current] = self
run_callbacks(:run)
end
diff --git a/activesupport/lib/active_support/reloader.rb \
b/activesupport/lib/active_support/reloader.rb index 2f81cd4f80..e751866a27 100644
--- a/activesupport/lib/active_support/reloader.rb
+++ b/activesupport/lib/active_support/reloader.rb
@@ -58,7 +58,7 @@ def self.reload!
prepare!
end
- def self.run! # :nodoc:
+ def self.run!(reset: false) # :nodoc:
if check!
super
else
--
2.35.0
["7.0-information-leak.patch" (application/octet-stream)]
From a2f695886379eb6d36540c7be41904877c49f713 Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Fri, 11 Feb 2022 13:09:30 +0100
Subject: [PATCH] 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]
---
.../action_dispatch/middleware/executor.rb | 2 +-
actionpack/test/dispatch/executor_test.rb | 21 ++++++++++
.../lib/active_support/execution_wrapper.rb | 42 +++++++++----------
.../isolated_execution_state.rb | 8 ++++
activesupport/lib/active_support/reloader.rb | 2 +-
5 files changed, 50 insertions(+), 25 deletions(-)
diff --git a/actionpack/lib/action_dispatch/middleware/executor.rb \
b/actionpack/lib/action_dispatch/middleware/executor.rb index 85326e313b..1878d64715 100644
--- a/actionpack/lib/action_dispatch/middleware/executor.rb
+++ b/actionpack/lib/action_dispatch/middleware/executor.rb
@@ -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! }
diff --git a/actionpack/test/dispatch/executor_test.rb \
b/actionpack/test/dispatch/executor_test.rb index 5b8be39b6d..d0bf574009 100644
--- a/actionpack/test/dispatch/executor_test.rb
+++ b/actionpack/test/dispatch/executor_test.rb
@@ -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"] })
diff --git a/activesupport/lib/active_support/execution_wrapper.rb \
b/activesupport/lib/active_support/execution_wrapper.rb index 87d90839ca..5a4a9b2c60 100644
--- a/activesupport/lib/active_support/execution_wrapper.rb
+++ b/activesupport/lib/active_support/execution_wrapper.rb
@@ -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:
diff --git a/activesupport/lib/active_support/isolated_execution_state.rb \
b/activesupport/lib/active_support/isolated_execution_state.rb index 7e14b9b4f6..d6322ff20a \
100644
--- a/activesupport/lib/active_support/isolated_execution_state.rb
+++ b/activesupport/lib/active_support/isolated_execution_state.rb
@@ -37,6 +37,14 @@ def []=(key, value)
current[key] = value
end
+ def key?(key)
+ current.key?(key)
+ end
+
+ def delete(key)
+ current.delete(key)
+ end
+
def clear
current.clear
end
diff --git a/activesupport/lib/active_support/reloader.rb \
b/activesupport/lib/active_support/reloader.rb index 2f81cd4f80..e751866a27 100644
--- a/activesupport/lib/active_support/reloader.rb
+++ b/activesupport/lib/active_support/reloader.rb
@@ -58,7 +58,7 @@ def self.reload!
prepare!
end
- def self.run! # :nodoc:
+ def self.run!(reset: false) # :nodoc:
if check!
super
else
--
2.35.0
["5.2-information-leak.patch" (application/octet-stream)]
From ed9175aa07f7c4fe59682a7b661d478411242a4a Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Fri, 11 Feb 2022 13:09:30 +0100
Subject: [PATCH] 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]
---
.../action_dispatch/middleware/executor.rb | 2 +-
actionpack/test/dispatch/executor_test.rb | 21 ++++++++++++++
.../lib/active_support/execution_wrapper.rb | 29 ++++++++++---------
activesupport/lib/active_support/reloader.rb | 2 +-
4 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/actionpack/lib/action_dispatch/middleware/executor.rb \
b/actionpack/lib/action_dispatch/middleware/executor.rb index 129b18d3d9..a32f916260 100644
--- a/actionpack/lib/action_dispatch/middleware/executor.rb
+++ b/actionpack/lib/action_dispatch/middleware/executor.rb
@@ -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! }
diff --git a/actionpack/test/dispatch/executor_test.rb \
b/actionpack/test/dispatch/executor_test.rb index 8eb6450385..e19e3bea32 100644
--- a/actionpack/test/dispatch/executor_test.rb
+++ b/actionpack/test/dispatch/executor_test.rb
@@ -119,6 +119,27 @@ def test_callbacks_execute_in_shared_context
assert !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"] })
diff --git a/activesupport/lib/active_support/execution_wrapper.rb \
b/activesupport/lib/active_support/execution_wrapper.rb index f48c586cad..77f41d94a2 100644
--- a/activesupport/lib/active_support/execution_wrapper.rb
+++ b/activesupport/lib/active_support/execution_wrapper.rb
@@ -62,18 +62,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 = active.delete(Thread.current)
+ 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
@@ -102,11 +105,11 @@ def self.inherited(other) # :nodoc:
self.active = Concurrent::Hash.new
def self.active? # :nodoc:
- @active[Thread.current]
+ @active.key?(Thread.current)
end
def run! # :nodoc:
- self.class.active[Thread.current] = true
+ self.class.active[Thread.current] = self
run_callbacks(:run)
end
diff --git a/activesupport/lib/active_support/reloader.rb \
b/activesupport/lib/active_support/reloader.rb index b26d9c3665..5acece49b7 100644
--- a/activesupport/lib/active_support/reloader.rb
+++ b/activesupport/lib/active_support/reloader.rb
@@ -59,7 +59,7 @@ def self.reload!
prepare!
end
- def self.run! # :nodoc:
+ def self.run!(reset: false) # :nodoc:
if check!
super
else
--
2.35.0
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic