Fix HTTP response splitting in CGI.
Backported from upstream Ruby 2.7.7, commit:
<7cf697179d>
Test "CGICookieTest#test_cgi_cookie_new_with_domain" was adjusted to
deal with Ruby 2.5 not allowing String as key with double splat operator.
Resolves: CVE-2021-33621
This commit is contained in:
Jarek Prokop
parent
59f3e8c0e5
commit
070d6a38cc
@ -0,0 +1,328 @@
|
|||||||
|
From 8fc4b4792919c627183f4ddb6dc256aae49eb738 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Hiroshi SHIBATA <hsbt@ruby-lang.org>
|
||||||
|
Date: Tue, 22 Nov 2022 13:48:18 +0900
|
||||||
|
Subject: [PATCH] Fix CVE-2021-33621 HTTP response splitting in CGI.
|
||||||
|
|
||||||
|
Backported from upstream Ruby, commit:
|
||||||
|
https://github.com/ruby/ruby/commit/7cf697179dab52b0d024543304f4d3ab5fa5e847
|
||||||
|
|
||||||
|
Test "CGICookieTest#test_cgi_cookie_new_with_domain" was adjusted to
|
||||||
|
deal with Ruby 2.5 not allowing String with double splat operator.
|
||||||
|
|
||||||
|
==== Original commit message
|
||||||
|
Merge CGI-0.1.0.2
|
||||||
|
---
|
||||||
|
lib/cgi/cookie.rb | 51 ++++++++++++++++-------
|
||||||
|
lib/cgi/core.rb | 45 ++++++++++++--------
|
||||||
|
test/cgi/test_cgi_cookie.rb | 82 +++++++++++++++++++++++++++++++++++++
|
||||||
|
test/cgi/test_cgi_header.rb | 8 ++++
|
||||||
|
4 files changed, 154 insertions(+), 32 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/lib/cgi/cookie.rb b/lib/cgi/cookie.rb
|
||||||
|
index 009566b..f26f015 100644
|
||||||
|
--- a/lib/cgi/cookie.rb
|
||||||
|
+++ b/lib/cgi/cookie.rb
|
||||||
|
@@ -40,6 +40,10 @@ class CGI
|
||||||
|
class Cookie < Array
|
||||||
|
@@accept_charset="UTF-8" unless defined?(@@accept_charset)
|
||||||
|
|
||||||
|
+ TOKEN_RE = %r"\A[[!-~]&&[^()<>@,;:\\\"/?=\[\]{}]]+\z"
|
||||||
|
+ PATH_VALUE_RE = %r"\A[[ -~]&&[^;]]*\z"
|
||||||
|
+ DOMAIN_VALUE_RE = %r"\A(?<label>(?!-)[-A-Za-z0-9]+(?<!-))(?:\.\g<label>)*\z"
|
||||||
|
+
|
||||||
|
# Create a new CGI::Cookie object.
|
||||||
|
#
|
||||||
|
# :call-seq:
|
||||||
|
@@ -72,9 +76,8 @@ class CGI
|
||||||
|
@domain = nil
|
||||||
|
@expires = nil
|
||||||
|
if name.kind_of?(String)
|
||||||
|
- @name = name
|
||||||
|
- %r|^(.*/)|.match(ENV["SCRIPT_NAME"])
|
||||||
|
- @path = ($1 or "")
|
||||||
|
+ self.name = name
|
||||||
|
+ self.path = (%r|\A(.*/)| =~ ENV["SCRIPT_NAME"] ? $1 : "")
|
||||||
|
@secure = false
|
||||||
|
@httponly = false
|
||||||
|
return super(value)
|
||||||
|
@@ -85,16 +88,11 @@ class CGI
|
||||||
|
raise ArgumentError, "`name' required"
|
||||||
|
end
|
||||||
|
|
||||||
|
- @name = options["name"]
|
||||||
|
+ self.name = options["name"]
|
||||||
|
value = Array(options["value"])
|
||||||
|
# simple support for IE
|
||||||
|
- if options["path"]
|
||||||
|
- @path = options["path"]
|
||||||
|
- else
|
||||||
|
- %r|^(.*/)|.match(ENV["SCRIPT_NAME"])
|
||||||
|
- @path = ($1 or "")
|
||||||
|
- end
|
||||||
|
- @domain = options["domain"]
|
||||||
|
+ self.path = options["path"] || (%r|\A(.*/)| =~ ENV["SCRIPT_NAME"] ? $1 : "")
|
||||||
|
+ self.domain = options["domain"]
|
||||||
|
@expires = options["expires"]
|
||||||
|
@secure = options["secure"] == true
|
||||||
|
@httponly = options["httponly"] == true
|
||||||
|
@@ -102,12 +100,35 @@ class CGI
|
||||||
|
super(value)
|
||||||
|
end
|
||||||
|
|
||||||
|
- # Name of this cookie, as a +String+
|
||||||
|
- attr_accessor :name
|
||||||
|
+ attr_reader :name
|
||||||
|
+ # Set name of this cookie
|
||||||
|
+ def name=(str)
|
||||||
|
+ if str and !TOKEN_RE.match?(str)
|
||||||
|
+ raise ArgumentError, "invalid name: #{str.dump}"
|
||||||
|
+ end
|
||||||
|
+ @name = str
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
# Path for which this cookie applies, as a +String+
|
||||||
|
- attr_accessor :path
|
||||||
|
+ attr_reader :path
|
||||||
|
+ # Set path for which this cookie applies
|
||||||
|
+ def path=(str)
|
||||||
|
+ if str and !PATH_VALUE_RE.match?(str)
|
||||||
|
+ raise ArgumentError, "invalid path: #{str.dump}"
|
||||||
|
+ end
|
||||||
|
+ @path = str
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
# Domain for which this cookie applies, as a +String+
|
||||||
|
- attr_accessor :domain
|
||||||
|
+ attr_reader :domain
|
||||||
|
+ # Set domain for which this cookie applies
|
||||||
|
+ def domain=(str)
|
||||||
|
+ if str and ((str = str.b).bytesize > 255 or !DOMAIN_VALUE_RE.match?(str))
|
||||||
|
+ raise ArgumentError, "invalid domain: #{str.dump}"
|
||||||
|
+ end
|
||||||
|
+ @domain = str
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
# Time at which this cookie expires, as a +Time+
|
||||||
|
attr_accessor :expires
|
||||||
|
# True if this cookie is secure; false otherwise
|
||||||
|
diff --git a/lib/cgi/core.rb b/lib/cgi/core.rb
|
||||||
|
index 9bd7798..7d8b223 100644
|
||||||
|
--- a/lib/cgi/core.rb
|
||||||
|
+++ b/lib/cgi/core.rb
|
||||||
|
@@ -188,17 +188,28 @@ class CGI
|
||||||
|
# Using #header with the HTML5 tag maker will create a <header> element.
|
||||||
|
alias :header :http_header
|
||||||
|
|
||||||
|
+ def _no_crlf_check(str)
|
||||||
|
+ if str
|
||||||
|
+ str = str.to_s
|
||||||
|
+ raise "A HTTP status or header field must not include CR and LF" if str =~ /[\r\n]/
|
||||||
|
+ str
|
||||||
|
+ else
|
||||||
|
+ nil
|
||||||
|
+ end
|
||||||
|
+ end
|
||||||
|
+ private :_no_crlf_check
|
||||||
|
+
|
||||||
|
def _header_for_string(content_type) #:nodoc:
|
||||||
|
buf = ''.dup
|
||||||
|
if nph?()
|
||||||
|
- buf << "#{$CGI_ENV['SERVER_PROTOCOL'] || 'HTTP/1.0'} 200 OK#{EOL}"
|
||||||
|
+ buf << "#{_no_crlf_check($CGI_ENV['SERVER_PROTOCOL']) || 'HTTP/1.0'} 200 OK#{EOL}"
|
||||||
|
buf << "Date: #{CGI.rfc1123_date(Time.now)}#{EOL}"
|
||||||
|
- buf << "Server: #{$CGI_ENV['SERVER_SOFTWARE']}#{EOL}"
|
||||||
|
+ buf << "Server: #{_no_crlf_check($CGI_ENV['SERVER_SOFTWARE'])}#{EOL}"
|
||||||
|
buf << "Connection: close#{EOL}"
|
||||||
|
end
|
||||||
|
- buf << "Content-Type: #{content_type}#{EOL}"
|
||||||
|
+ buf << "Content-Type: #{_no_crlf_check(content_type)}#{EOL}"
|
||||||
|
if @output_cookies
|
||||||
|
- @output_cookies.each {|cookie| buf << "Set-Cookie: #{cookie}#{EOL}" }
|
||||||
|
+ @output_cookies.each {|cookie| buf << "Set-Cookie: #{_no_crlf_check(cookie)}#{EOL}" }
|
||||||
|
end
|
||||||
|
return buf
|
||||||
|
end # _header_for_string
|
||||||
|
@@ -213,9 +224,9 @@ class CGI
|
||||||
|
## NPH
|
||||||
|
options.delete('nph') if defined?(MOD_RUBY)
|
||||||
|
if options.delete('nph') || nph?()
|
||||||
|
- protocol = $CGI_ENV['SERVER_PROTOCOL'] || 'HTTP/1.0'
|
||||||
|
+ protocol = _no_crlf_check($CGI_ENV['SERVER_PROTOCOL']) || 'HTTP/1.0'
|
||||||
|
status = options.delete('status')
|
||||||
|
- status = HTTP_STATUS[status] || status || '200 OK'
|
||||||
|
+ status = HTTP_STATUS[status] || _no_crlf_check(status) || '200 OK'
|
||||||
|
buf << "#{protocol} #{status}#{EOL}"
|
||||||
|
buf << "Date: #{CGI.rfc1123_date(Time.now)}#{EOL}"
|
||||||
|
options['server'] ||= $CGI_ENV['SERVER_SOFTWARE'] || ''
|
||||||
|
@@ -223,38 +234,38 @@ class CGI
|
||||||
|
end
|
||||||
|
## common headers
|
||||||
|
status = options.delete('status')
|
||||||
|
- buf << "Status: #{HTTP_STATUS[status] || status}#{EOL}" if status
|
||||||
|
+ buf << "Status: #{HTTP_STATUS[status] || _no_crlf_check(status)}#{EOL}" if status
|
||||||
|
server = options.delete('server')
|
||||||
|
- buf << "Server: #{server}#{EOL}" if server
|
||||||
|
+ buf << "Server: #{_no_crlf_check(server)}#{EOL}" if server
|
||||||
|
connection = options.delete('connection')
|
||||||
|
- buf << "Connection: #{connection}#{EOL}" if connection
|
||||||
|
+ buf << "Connection: #{_no_crlf_check(connection)}#{EOL}" if connection
|
||||||
|
type = options.delete('type')
|
||||||
|
- buf << "Content-Type: #{type}#{EOL}" #if type
|
||||||
|
+ buf << "Content-Type: #{_no_crlf_check(type)}#{EOL}" #if type
|
||||||
|
length = options.delete('length')
|
||||||
|
- buf << "Content-Length: #{length}#{EOL}" if length
|
||||||
|
+ buf << "Content-Length: #{_no_crlf_check(length)}#{EOL}" if length
|
||||||
|
language = options.delete('language')
|
||||||
|
- buf << "Content-Language: #{language}#{EOL}" if language
|
||||||
|
+ buf << "Content-Language: #{_no_crlf_check(language)}#{EOL}" if language
|
||||||
|
expires = options.delete('expires')
|
||||||
|
buf << "Expires: #{CGI.rfc1123_date(expires)}#{EOL}" if expires
|
||||||
|
## cookie
|
||||||
|
if cookie = options.delete('cookie')
|
||||||
|
case cookie
|
||||||
|
when String, Cookie
|
||||||
|
- buf << "Set-Cookie: #{cookie}#{EOL}"
|
||||||
|
+ buf << "Set-Cookie: #{_no_crlf_check(cookie)}#{EOL}"
|
||||||
|
when Array
|
||||||
|
arr = cookie
|
||||||
|
- arr.each {|c| buf << "Set-Cookie: #{c}#{EOL}" }
|
||||||
|
+ arr.each {|c| buf << "Set-Cookie: #{_no_crlf_check(c)}#{EOL}" }
|
||||||
|
when Hash
|
||||||
|
hash = cookie
|
||||||
|
- hash.each_value {|c| buf << "Set-Cookie: #{c}#{EOL}" }
|
||||||
|
+ hash.each_value {|c| buf << "Set-Cookie: #{_no_crlf_check(c)}#{EOL}" }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
if @output_cookies
|
||||||
|
- @output_cookies.each {|c| buf << "Set-Cookie: #{c}#{EOL}" }
|
||||||
|
+ @output_cookies.each {|c| buf << "Set-Cookie: #{_no_crlf_check(c)}#{EOL}" }
|
||||||
|
end
|
||||||
|
## other headers
|
||||||
|
options.each do |key, value|
|
||||||
|
- buf << "#{key}: #{value}#{EOL}"
|
||||||
|
+ buf << "#{_no_crlf_check(key)}: #{_no_crlf_check(value)}#{EOL}"
|
||||||
|
end
|
||||||
|
return buf
|
||||||
|
end # _header_for_hash
|
||||||
|
diff --git a/test/cgi/test_cgi_cookie.rb b/test/cgi/test_cgi_cookie.rb
|
||||||
|
index 985cc0d..7afff5e 100644
|
||||||
|
--- a/test/cgi/test_cgi_cookie.rb
|
||||||
|
+++ b/test/cgi/test_cgi_cookie.rb
|
||||||
|
@@ -60,6 +60,24 @@ class CGICookieTest < Test::Unit::TestCase
|
||||||
|
end
|
||||||
|
|
||||||
|
|
||||||
|
+ def test_cgi_cookie_new_with_domain
|
||||||
|
+ h = {'name'=>'name1', 'value'=>'value1'}
|
||||||
|
+ cookie = CGI::Cookie.new({'domain' => 'a.example.com'}.merge(h))
|
||||||
|
+ assert_equal('a.example.com', cookie.domain)
|
||||||
|
+
|
||||||
|
+ cookie = CGI::Cookie.new({'domain'=>'1.example.com'}.merge(h))
|
||||||
|
+ assert_equal('1.example.com', cookie.domain, 'enhanced by RFC 1123')
|
||||||
|
+
|
||||||
|
+ assert_raise(ArgumentError) {
|
||||||
|
+ CGI::Cookie.new({'domain'=>'-a.example.com'}.merge(h))
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ assert_raise(ArgumentError) {
|
||||||
|
+ CGI::Cookie.new({'domain'=>'a-.example.com'}.merge(h))
|
||||||
|
+ }
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
+
|
||||||
|
def test_cgi_cookie_scriptname
|
||||||
|
cookie = CGI::Cookie.new('name1', 'value1')
|
||||||
|
assert_equal('', cookie.path)
|
||||||
|
@@ -118,6 +136,70 @@ class CGICookieTest < Test::Unit::TestCase
|
||||||
|
end
|
||||||
|
|
||||||
|
|
||||||
|
+ def test_cgi_cookie_domain_injection_into_name
|
||||||
|
+ name = "a=b; domain=example.com;"
|
||||||
|
+ path = "/"
|
||||||
|
+ domain = "example.jp"
|
||||||
|
+ assert_raise(ArgumentError) do
|
||||||
|
+ CGI::Cookie.new('name' => name,
|
||||||
|
+ 'value' => "value",
|
||||||
|
+ 'domain' => domain,
|
||||||
|
+ 'path' => path)
|
||||||
|
+ end
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
+
|
||||||
|
+ def test_cgi_cookie_newline_injection_into_name
|
||||||
|
+ name = "a=b;\r\nLocation: http://example.com#"
|
||||||
|
+ path = "/"
|
||||||
|
+ domain = "example.jp"
|
||||||
|
+ assert_raise(ArgumentError) do
|
||||||
|
+ CGI::Cookie.new('name' => name,
|
||||||
|
+ 'value' => "value",
|
||||||
|
+ 'domain' => domain,
|
||||||
|
+ 'path' => path)
|
||||||
|
+ end
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
+
|
||||||
|
+ def test_cgi_cookie_multibyte_injection_into_name
|
||||||
|
+ name = "a=b;\u3042"
|
||||||
|
+ path = "/"
|
||||||
|
+ domain = "example.jp"
|
||||||
|
+ assert_raise(ArgumentError) do
|
||||||
|
+ CGI::Cookie.new('name' => name,
|
||||||
|
+ 'value' => "value",
|
||||||
|
+ 'domain' => domain,
|
||||||
|
+ 'path' => path)
|
||||||
|
+ end
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
+
|
||||||
|
+ def test_cgi_cookie_injection_into_path
|
||||||
|
+ name = "name"
|
||||||
|
+ path = "/; samesite=none"
|
||||||
|
+ domain = "example.jp"
|
||||||
|
+ assert_raise(ArgumentError) do
|
||||||
|
+ CGI::Cookie.new('name' => name,
|
||||||
|
+ 'value' => "value",
|
||||||
|
+ 'domain' => domain,
|
||||||
|
+ 'path' => path)
|
||||||
|
+ end
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
+
|
||||||
|
+ def test_cgi_cookie_injection_into_domain
|
||||||
|
+ name = "name"
|
||||||
|
+ path = "/"
|
||||||
|
+ domain = "example.jp; samesite=none"
|
||||||
|
+ assert_raise(ArgumentError) do
|
||||||
|
+ CGI::Cookie.new('name' => name,
|
||||||
|
+ 'value' => "value",
|
||||||
|
+ 'domain' => domain,
|
||||||
|
+ 'path' => path)
|
||||||
|
+ end
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
|
||||||
|
instance_methods.each do |method|
|
||||||
|
private method if method =~ /^test_(.*)/ && $1 != ENV['TEST']
|
||||||
|
diff --git a/test/cgi/test_cgi_header.rb b/test/cgi/test_cgi_header.rb
|
||||||
|
index bab2d03..ec2f4de 100644
|
||||||
|
--- a/test/cgi/test_cgi_header.rb
|
||||||
|
+++ b/test/cgi/test_cgi_header.rb
|
||||||
|
@@ -176,6 +176,14 @@ class CGIHeaderTest < Test::Unit::TestCase
|
||||||
|
end
|
||||||
|
|
||||||
|
|
||||||
|
+ def test_cgi_http_header_crlf_injection
|
||||||
|
+ cgi = CGI.new
|
||||||
|
+ assert_raise(RuntimeError) { cgi.http_header("text/xhtml\r\nBOO") }
|
||||||
|
+ assert_raise(RuntimeError) { cgi.http_header("type" => "text/xhtml\r\nBOO") }
|
||||||
|
+ assert_raise(RuntimeError) { cgi.http_header("status" => "200 OK\r\nBOO") }
|
||||||
|
+ assert_raise(RuntimeError) { cgi.http_header("location" => "text/xhtml\r\nBOO") }
|
||||||
|
+ end
|
||||||
|
+
|
||||||
|
|
||||||
|
instance_methods.each do |method|
|
||||||
|
private method if method =~ /^test_(.*)/ && $1 != ENV['TEST']
|
||||||
|
--
|
||||||
|
2.41.0
|
||||||
|
|
||||||
10
ruby.spec
10
ruby.spec
@ -208,6 +208,11 @@ Patch36: ruby-3.1.3-Fix-for-tzdata-2022g.patch
|
|||||||
# Bypass git submodule test failure on Git >= 2.38.1.
|
# Bypass git submodule test failure on Git >= 2.38.1.
|
||||||
# https://github.com/ruby/ruby/pull/6587
|
# https://github.com/ruby/ruby/pull/6587
|
||||||
Patch37: ruby-3.2.0-git-2.38.1-fix-rubygems-test.patch
|
Patch37: ruby-3.2.0-git-2.38.1-fix-rubygems-test.patch
|
||||||
|
# CVE-2021-33621: HTTP response splitting in CGI.
|
||||||
|
# Backported from:
|
||||||
|
# https://github.com/ruby/ruby/commit/7cf697179dab52b0d024543304f4d3ab5fa5e847
|
||||||
|
Patch38: ruby-2.7.7-Fix-CVE-2021-33621-HTTP-response-splitting-in-CGI.patch
|
||||||
|
|
||||||
|
|
||||||
Requires: %{name}-libs%{?_isa} = %{version}-%{release}
|
Requires: %{name}-libs%{?_isa} = %{version}-%{release}
|
||||||
Suggests: rubypick
|
Suggests: rubypick
|
||||||
@ -615,6 +620,7 @@ sed -i 's/"evaluation\/incorrect_words.yaml"\.freeze, //' \
|
|||||||
%patch35 -p1
|
%patch35 -p1
|
||||||
%patch36 -p1
|
%patch36 -p1
|
||||||
%patch37 -p1
|
%patch37 -p1
|
||||||
|
%patch38 -p1
|
||||||
|
|
||||||
# Provide an example of usage of the tapset:
|
# Provide an example of usage of the tapset:
|
||||||
cp -a %{SOURCE3} .
|
cp -a %{SOURCE3} .
|
||||||
@ -1167,6 +1173,10 @@ OPENSSL_SYSTEM_CIPHERS_OVERRIDE=xyz_nonexistent_file OPENSSL_CONF='' \
|
|||||||
%{gem_dir}/specifications/xmlrpc-%{xmlrpc_version}.gemspec
|
%{gem_dir}/specifications/xmlrpc-%{xmlrpc_version}.gemspec
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Mon Jun 12 2023 Jarek Prokop <jprokop@redhat.com> - 2.5.9-111
|
||||||
|
- Fix HTTP response splitting in CGI.
|
||||||
|
Resolves: CVE-2021-33621
|
||||||
|
|
||||||
* Thu May 25 2023 Todd Zullinger <tmz@pobox.com> - 2.5.9-111
|
* Thu May 25 2023 Todd Zullinger <tmz@pobox.com> - 2.5.9-111
|
||||||
- Fix rdoc parsing of nil text tokens.
|
- Fix rdoc parsing of nil text tokens.
|
||||||
Resolves: rhbz#2210326
|
Resolves: rhbz#2210326
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user