71 lines
3.1 KiB
Diff
71 lines
3.1 KiB
Diff
|
From 6d3e49f128c2db4cb157e058effe07781b0a66e4 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Zack Deveau <zack.ref@gmail.com>
|
||
|
|
Date: Thu, 11 May 2023 16:55:01 -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://discuss.rubyonrails.org/t/cve-2023-28362-possible-xss-via-user-supplied-values-to-redirect-to/83132
|
||
|
|
|
||
|
|
format
|
||
|
|
---
|
||
|
|
.../action_controller/metal/redirecting.rb | 19 ++++++++++++++++++-
|
||
|
|
actionpack/test/controller/redirect_test.rb | 17 +++++++++++++++++
|
||
|
|
2 files changed, 35 insertions(+), 1 deletion(-)
|
||
|
|
|
||
|
|
diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb
|
||
|
|
index 0409ba7026..830b94c092 100644
|
||
|
|
--- a/actionpack/lib/action_controller/metal/redirecting.rb
|
||
|
|
+++ b/actionpack/lib/action_controller/metal/redirecting.rb
|
||
|
|
@@ -4,6 +4,8 @@ module ActionController
|
||
|
|
module Redirecting
|
||
|
|
extend ActiveSupport::Concern
|
||
|
|
|
||
|
|
+ ILLEGAL_HEADER_VALUE_REGEX = /[\x00-\x08\x0A-\x1F]/.freeze
|
||
|
|
+
|
||
|
|
include AbstractController::Logger
|
||
|
|
include ActionController::UrlFor
|
||
|
|
|
||
|
|
@@ -86,7 +88,11 @@ def redirect_to(options = {}, response_options = {})
|
||
|
|
allow_other_host = response_options.delete(:allow_other_host) { _allow_other_host }
|
||
|
|
|
||
|
|
self.status = _extract_redirect_to_status(options, response_options)
|
||
|
|
- self.location = _enforce_open_redirect_protection(_compute_redirect_to_location(request, options), allow_other_host: allow_other_host)
|
||
|
|
+
|
||
|
|
+ redirect_to_location = _compute_redirect_to_location(request, options)
|
||
|
|
+ _ensure_url_is_http_header_safe(redirect_to_location)
|
||
|
|
+
|
||
|
|
+ self.location = _enforce_open_redirect_protection(redirect_to_location, allow_other_host: allow_other_host)
|
||
|
|
self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>"
|
||
|
|
end
|
||
|
|
|
||
|
|
@@ -204,5 +210,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
|