Treat comma as normal character in HTTP::Cookie.cookie_value_to_hash

As pointed out in CVE-2016-7401, treating comma in a Cookie header value
as separator may cause security problems.
This commit is contained in:
Akinori MUSHA 2016-09-30 19:20:39 +09:00
parent 1c4a7bbe4b
commit 7f94a9e5d9
2 changed files with 25 additions and 13 deletions

View file

@ -50,7 +50,7 @@ class HTTP::Cookie::Scanner < StringScanner
}
end
def scan_value
def scan_value(comma_as_separator = false)
''.tap { |s|
case
when scan(/[^,;"]+/)
@ -59,7 +59,9 @@ class HTTP::Cookie::Scanner < StringScanner
# RFC 6265 2.2
# A cookie-value may be DQUOTE'd.
s << scan_dquoted
when check(/;|#{RE_COOKIE_COMMA}/o)
when check(/;/)
break
when comma_as_separator && check(RE_COOKIE_COMMA)
break
else
s << getch
@ -68,12 +70,12 @@ class HTTP::Cookie::Scanner < StringScanner
}
end
def scan_name_value
def scan_name_value(comma_as_separator = false)
name = scan_name
if skip(/\=/)
value = scan_value
value = scan_value(comma_as_separator)
else
scan_value
scan_value(comma_as_separator)
value = nil
end
[name, value]
@ -159,7 +161,7 @@ class HTTP::Cookie::Scanner < StringScanner
skip_wsp
name, value = scan_name_value
name, value = scan_name_value(true)
if value.nil?
@logger.warn("Cookie definition lacks a name-value pair.") if @logger
elsif name.empty?
@ -176,7 +178,7 @@ class HTTP::Cookie::Scanner < StringScanner
break
when skip(/;/)
skip_wsp
aname, avalue = scan_name_value
aname, avalue = scan_name_value(true)
next if aname.empty? || value.nil?
aname.downcase!
case aname
@ -218,13 +220,12 @@ class HTTP::Cookie::Scanner < StringScanner
until eos?
skip_wsp
name, value = scan_name_value
# Do not treat comma in a Cookie header value as separator; see CVE-2016-7401
name, value = scan_name_value(false)
yield name, value if value
# The comma is used as separator for concatenating multiple
# values of a header.
skip(/[;,]/)
skip(/;/)
end
end
end

View file

@ -441,17 +441,28 @@ class TestHTTPCookie < Test::Unit::TestCase
['Bar', 'value 2'],
['Baz', 'value3'],
['Bar', 'value"4'],
['Quux', 'x, value=5'],
]
cookie_value = HTTP::Cookie.cookie_value(pairs.map { |name, value|
HTTP::Cookie.new(:name => name, :value => value)
})
assert_equal 'Foo=value1; Bar="value 2"; Baz=value3; Bar="value\\"4"', cookie_value
assert_equal 'Foo=value1; Bar="value 2"; Baz=value3; Bar="value\\"4"; Quux="x, value=5"', cookie_value
hash = HTTP::Cookie.cookie_value_to_hash(cookie_value)
assert_equal 3, hash.size
assert_equal pairs.map(&:first).uniq.size, hash.size
hash.each_pair { |name, value|
_, pvalue = pairs.assoc(name)
assert_equal pvalue, value
}
# Do not treat comma in a Cookie header value as separator; see CVE-2016-7401
hash = HTTP::Cookie.cookie_value_to_hash('Quux=x, value=5; Foo=value1; Bar="value 2"; Baz=value3; Bar="value\\"4"')
assert_equal pairs.map(&:first).uniq.size, hash.size
hash.each_pair { |name, value|
_, pvalue = pairs.assoc(name)