Skip to content

Commit 0fccfb9

Browse files
StefSchenkelaarstenderlove
authored andcommitted
Fix invalid forwarded host vulnerability
Prior to this commit, it was possible to pass an unvalidated host through the `X-Forwarded-Host` header. If the value of the header was prefixed with a invalid domain character (for example a `/`), it was always accepted as the actual host of that request. Since this host is used for all url helpers, an attacker could change generated links and redirects. If the header is set to `X-Forwarded-Host: //evil.hacker`, a redirect will be send to `https:////evil.hacker/`. Browsers will ignore these four slashes and redirect the user. [CVE-2021-44528]
1 parent d12eca1 commit 0fccfb9

File tree

2 files changed

+91
-8
lines changed

2 files changed

+91
-8
lines changed

Diff for: actionpack/lib/action_dispatch/middleware/host_authorization.rb

+3-7
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def sanitize_regexp(host)
5252

5353
def sanitize_string(host)
5454
if host.start_with?(".")
55-
/\A(.+\.)?#{Regexp.escape(host[1..-1])}\z/i
55+
/\A([a-z0-9-]+\.)?#{Regexp.escape(host[1..-1])}\z/i
5656
else
5757
/\A#{Regexp.escape host}\z/i
5858
end
@@ -120,13 +120,9 @@ def call(env)
120120
end
121121

122122
private
123-
HOSTNAME = /[a-z0-9.-]+|\[[a-f0-9]*:[a-f0-9.:]+\]/i
124-
VALID_ORIGIN_HOST = /\A(#{HOSTNAME})(?::\d+)?\z/
125-
VALID_FORWARDED_HOST = /(?:\A|,[ ]?)(#{HOSTNAME})(?::\d+)?\z/
126-
127123
def authorized?(request)
128-
origin_host = request.get_header("HTTP_HOST")&.slice(VALID_ORIGIN_HOST, 1) || ""
129-
forwarded_host = request.x_forwarded_host&.slice(VALID_FORWARDED_HOST, 1) || ""
124+
origin_host = request.get_header("HTTP_HOST")
125+
forwarded_host = request.x_forwarded_host&.split(/,\s?/)&.last
130126

131127
@permissions.allows?(origin_host) && (forwarded_host.blank? || @permissions.allows?(forwarded_host))
132128
end

Diff for: actionpack/test/dispatch/host_authorization_test.rb

+88-1
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,44 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
167167
assert_match "Blocked host: 127.0.0.1", response.body
168168
end
169169

170+
test "blocks requests with spoofed relative X-FORWARDED-HOST" do
171+
@app = ActionDispatch::HostAuthorization.new(App, ["www.example.com"])
172+
173+
get "/", env: {
174+
"HTTP_X_FORWARDED_HOST" => "//randomhost.com",
175+
"HOST" => "www.example.com",
176+
"action_dispatch.show_detailed_exceptions" => true
177+
}
178+
179+
assert_response :forbidden
180+
assert_match "Blocked host: //randomhost.com", response.body
181+
end
182+
183+
test "forwarded secondary hosts are allowed when permitted" do
184+
@app = ActionDispatch::HostAuthorization.new(App, ".domain.com")
185+
186+
get "/", env: {
187+
"HTTP_X_FORWARDED_HOST" => "example.com, my-sub.domain.com",
188+
"HOST" => "domain.com",
189+
}
190+
191+
assert_response :ok
192+
assert_equal "Success", body
193+
end
194+
195+
test "forwarded secondary hosts are blocked when mismatch" do
196+
@app = ActionDispatch::HostAuthorization.new(App, "domain.com")
197+
198+
get "/", env: {
199+
"HTTP_X_FORWARDED_HOST" => "domain.com, evil.com",
200+
"HOST" => "domain.com",
201+
"action_dispatch.show_detailed_exceptions" => true
202+
}
203+
204+
assert_response :forbidden
205+
assert_match "Blocked host: evil.com", response.body
206+
end
207+
170208
test "does not consider IP addresses in X-FORWARDED-HOST spoofed when disabled" do
171209
@app = ActionDispatch::HostAuthorization.new(App, nil)
172210

@@ -205,18 +243,67 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
205243
assert_match "Blocked host: sub.domain.com", response.body
206244
end
207245

246+
test "sub-sub domains should not be permitted" do
247+
@app = ActionDispatch::HostAuthorization.new(App, ".domain.com")
248+
249+
get "/", env: {
250+
"HOST" => "secondary.sub.domain.com",
251+
"action_dispatch.show_detailed_exceptions" => true
252+
}
253+
254+
assert_response :forbidden
255+
assert_match "Blocked host: secondary.sub.domain.com", response.body
256+
end
257+
208258
test "forwarded hosts are allowed when permitted" do
209259
@app = ActionDispatch::HostAuthorization.new(App, ".domain.com")
210260

211261
get "/", env: {
212-
"HTTP_X_FORWARDED_HOST" => "sub.domain.com",
262+
"HTTP_X_FORWARDED_HOST" => "my-sub.domain.com",
213263
"HOST" => "domain.com",
214264
}
215265

216266
assert_response :ok
217267
assert_equal "Success", body
218268
end
219269

270+
test "lots of NG hosts" do
271+
ng_hosts = [
272+
"hacker%E3%80%82com",
273+
"hacker%00.com",
274+
"www.theirsite.com@yoursite.com",
275+
"hacker.com/test/",
276+
"hacker%252ecom",
277+
".hacker.com",
278+
"/\/\/hacker.com/",
279+
"/hacker.com",
280+
"../hacker.com",
281+
".hacker.com",
282+
"@hacker.com",
283+
"hacker.com",
284+
"hacker.com%23@example.com",
285+
"hacker.com/.jpg",
286+
"hacker.com\texample.com/",
287+
"hacker.com/example.com",
288+
"hacker.com\@example.com",
289+
"hacker.com/example.com",
290+
"hacker.com/"
291+
]
292+
293+
@app = ActionDispatch::HostAuthorization.new(App, "example.com")
294+
295+
ng_hosts.each do |host|
296+
get "/", env: {
297+
"HTTP_X_FORWARDED_HOST" => host,
298+
"HOST" => "example.com",
299+
"action_dispatch.show_detailed_exceptions" => true
300+
}
301+
302+
assert_response :forbidden
303+
assert_match "Blocked host: #{host}", response.body
304+
end
305+
end
306+
220307
test "exclude matches allow any host" do
221308
@app = ActionDispatch::HostAuthorization.new(App, "only.com", exclude: ->(req) { req.path == "/foo" })
222309

0 commit comments

Comments
 (0)