170 lines
5.0 KiB
Diff
170 lines
5.0 KiB
Diff
From 30107a4797f14227568913499a9a0bb4285de63b Mon Sep 17 00:00:00 2001
|
|
From: Nobuyoshi Nakada <nobu@ruby-lang.org>
|
|
Date: Tue, 16 Aug 2022 18:36:12 +0900
|
|
Subject: [PATCH] Check cookie name/path/domain characters
|
|
|
|
https://hackerone.com/reports/1204977
|
|
---
|
|
lib/cgi/cookie.rb | 44 ++++++++++++++++++++-----
|
|
test/cgi/test_cgi_cookie.rb | 64 +++++++++++++++++++++++++++++++++++++
|
|
2 files changed, 100 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/lib/cgi/cookie.rb b/lib/cgi/cookie.rb
|
|
index 6b0d89c..4b11a6a 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-z][-A-Za-z0-9]*[A-Za-z0-9])(?:\.\g<label>)*\z"
|
|
+
|
|
# Create a new CGI::Cookie object.
|
|
#
|
|
# :call-seq:
|
|
@@ -72,8 +76,8 @@ class CGI
|
|
@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)
|
|
@@ -84,11 +88,11 @@ class CGI
|
|
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
|
|
@@ -97,11 +101,35 @@ class CGI
|
|
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/test/cgi/test_cgi_cookie.rb b/test/cgi/test_cgi_cookie.rb
|
|
index 985cc0d..2f09d0f 100644
|
|
--- a/test/cgi/test_cgi_cookie.rb
|
|
+++ b/test/cgi/test_cgi_cookie.rb
|
|
@@ -118,6 +118,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']
|
|
--
|
|
2.33.0
|
|
|