Skip to content

Commit

Permalink
Merge CGI-0.1.0.2
Browse files Browse the repository at this point in the history
  • Loading branch information
hsbt authored and unak committed Nov 24, 2022
1 parent 7b413c1 commit 7cf6971
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 26 deletions.
44 changes: 36 additions & 8 deletions lib/cgi/cookie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"

This comment has been minimized.

Copy link
@heaven

heaven Dec 13, 2022

This breaks wildcard domains in cookies, such as .example.com. This has been fixed in 3 and probably should be backported to 2.7.

This comment has been minimized.

Copy link
@hsbt

This comment has been minimized.

Copy link
@heaven

heaven Dec 13, 2022

Thank you 🙏


# Create a new CGI::Cookie object.
#
# :call-seq:
Expand Down Expand Up @@ -72,8 +76,8 @@ def initialize(name = "", *value)
@domain = nil
@expires = nil
if name.kind_of?(String)
@name = name
@path = (%r|\A(.*/)| =~ ENV["SCRIPT_NAME"] ? $1 : "")
self.name = name
self.path = (%r|\A(.*/)| =~ ENV["SCRIPT_NAME"] ? $1 : "")
@secure = false
@httponly = false
return super(value)
Expand All @@ -84,11 +88,11 @@ def initialize(name = "", *value)
raise ArgumentError, "`name' required"
end

@name = options["name"]
self.name = options["name"]
value = Array(options["value"])
# simple support for IE
@path = options["path"] || (%r|\A(.*/)| =~ ENV["SCRIPT_NAME"] ? $1 : "")
@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
Expand All @@ -97,11 +101,35 @@ def initialize(name = "", *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
Expand Down
45 changes: 28 additions & 17 deletions lib/cgi/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,17 +188,28 @@ def http_header(options='text/html')
# 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
Expand All @@ -213,48 +224,48 @@ def _header_for_hash(options) #:nodoc:
## 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'] || ''
options['connection'] ||= 'close'
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
Expand Down
2 changes: 1 addition & 1 deletion lib/cgi/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
class CGI
VERSION = "0.1.0.1"
VERSION = "0.1.0.2"
end
82 changes: 82 additions & 0 deletions test/cgi/test_cgi_cookie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@ def test_cgi_cookie_new_complex
end


def test_cgi_cookie_new_with_domain
h = {'name'=>'name1', 'value'=>'value1'}
cookie = CGI::Cookie.new('domain'=>'a.example.com', **h)
assert_equal('a.example.com', cookie.domain)

cookie = CGI::Cookie.new('domain'=>'1.example.com', **h)
assert_equal('1.example.com', cookie.domain, 'enhanced by RFC 1123')

assert_raise(ArgumentError) {
CGI::Cookie.new('domain'=>'-a.example.com', **h)
}

assert_raise(ArgumentError) {
CGI::Cookie.new('domain'=>'a-.example.com', **h)
}
end


def test_cgi_cookie_scriptname
cookie = CGI::Cookie.new('name1', 'value1')
assert_equal('', cookie.path)
Expand Down Expand Up @@ -118,6 +136,70 @@ def test_cgi_cookie_arrayinterface
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']
Expand Down
8 changes: 8 additions & 0 deletions test/cgi/test_cgi_header.rb
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ def test_cgi_http_header_nph
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']
Expand Down

0 comments on commit 7cf6971

Please sign in to comment.