1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
|
From e9015f91dd685472f915f8aa1eb18b0e0763e013 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 ++++++++++---------
3 files changed, 38 insertions(+), 14 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
--
2.30.2
|
