72 lines
3.1 KiB
Diff
72 lines
3.1 KiB
Diff
From 1c3f93d1e90a3475f9ae2377ead25ccf11f71441 Mon Sep 17 00:00:00 2001
|
|
From: Zack Deveau <zack.ref@gmail.com>
|
|
Date: Fri, 12 May 2023 13:04:22 -0400
|
|
Subject: [PATCH] Added check for illegal HTTP header value in redirect_to
|
|
|
|
The set of legal characters for an HTTP header value is described
|
|
in https://datatracker.ietf.org/doc/html/rfc7230\#section-3.2.6.
|
|
|
|
This commit adds a check to redirect_to that ensures the
|
|
provided URL does not contain any of the illegal characters.
|
|
|
|
Downstream consumers of the resulting Location response header
|
|
may remove the header if it does not comply with the RFC.
|
|
This can result in a cross site scripting (XSS) vector by
|
|
allowing for the redirection page to sit idle waiting
|
|
for user interaction with the provided malicious link.
|
|
|
|
[CVE-2023-28362]
|
|
|
|
Origin: https://github.com/rails/rails/commit/1c3f93d1e90a3475f9ae2377ead25ccf11f71441
|
|
|
|
---
|
|
.../action_controller/metal/redirecting.rb | 21 ++++++++++++++++++-
|
|
actionpack/test/controller/redirect_test.rb | 17 +++++++++++++++
|
|
2 files changed, 37 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb
|
|
index 11d462855d064..fdd3f9dc44149 100644
|
|
--- a/actionpack/lib/action_controller/metal/redirecting.rb
|
|
+++ b/actionpack/lib/action_controller/metal/redirecting.rb
|
|
@@ -7,6 +7,10 @@ module Redirecting
|
|
include AbstractController::Logger
|
|
include ActionController::UrlFor
|
|
|
|
+ ILLEGAL_HEADER_VALUE_REGEX = /[\x00-\x08\x0A-\x1F]/.freeze
|
|
+
|
|
+ class UnsafeRedirectError < StandardError; end
|
|
+
|
|
# Redirects the browser to the target specified in +options+. This parameter can be any one of:
|
|
#
|
|
# * <tt>Hash</tt> - The URL will be generated by calling url_for with the +options+.
|
|
@@ -60,7 +64,11 @@ def redirect_to(options = {}, response_options = {})
|
|
raise AbstractController::DoubleRenderError if response_body
|
|
|
|
self.status = _extract_redirect_to_status(options, response_options)
|
|
- self.location = _compute_redirect_to_location(request, options)
|
|
+
|
|
+ redirect_to_location = _compute_redirect_to_location(request, options)
|
|
+ _ensure_url_is_http_header_safe(redirect_to_location)
|
|
+
|
|
+ self.location = redirect_to_location
|
|
self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>"
|
|
end
|
|
|
|
@@ -129,5 +137,16 @@ def _url_host_allowed?(url)
|
|
rescue ArgumentError, URI::Error
|
|
false
|
|
end
|
|
+
|
|
+ def _ensure_url_is_http_header_safe(url)
|
|
+ # Attempt to comply with the set of valid token characters
|
|
+ # defined for an HTTP header value in
|
|
+ # https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6
|
|
+ if url.match(ILLEGAL_HEADER_VALUE_REGEX)
|
|
+ msg = "The redirect URL #{url} contains one or more illegal HTTP header field character. " \
|
|
+ "Set of legal characters defined in https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.6"
|
|
+ raise UnsafeRedirectError, msg
|
|
+ end
|
|
+ end
|
|
end
|
|
end
|