Issue #22796: HTTP cookie parsing is now stricter, in order to protect against potential injection attacks.
diff --git a/Lib/http/cookies.py b/Lib/http/cookies.py
index a6de6d5..7ec9274 100644
--- a/Lib/http/cookies.py
+++ b/Lib/http/cookies.py
@@ -533,10 +533,17 @@
return
def __parse_string(self, str, patt=_CookiePattern):
- i = 0 # Our starting point
- n = len(str) # Length of string
- M = None # current morsel
+ i = 0 # Our starting point
+ n = len(str) # Length of string
+ parsed_items = [] # Parsed (type, key, value) triples
+ morsel_seen = False # A key=value pair was previously encountered
+ TYPE_ATTRIBUTE = 1
+ TYPE_KEYVALUE = 2
+
+ # We first parse the whole cookie string and reject it if it's
+ # syntactically invalid (this helps avoid some classes of injection
+ # attacks).
while 0 <= i < n:
# Start looking for a cookie
match = patt.match(str, i)
@@ -547,22 +554,41 @@
key, value = match.group("key"), match.group("val")
i = match.end(0)
- # Parse the key, value in case it's metainfo
if key[0] == "$":
- # We ignore attributes which pertain to the cookie
- # mechanism as a whole. See RFC 2109.
- # (Does anyone care?)
- if M:
- M[key[1:]] = value
+ if not morsel_seen:
+ # We ignore attributes which pertain to the cookie
+ # mechanism as a whole, such as "$Version".
+ # See RFC 2965. (Does anyone care?)
+ continue
+ parsed_items.append((TYPE_ATTRIBUTE, key[1:], value))
elif key.lower() in Morsel._reserved:
- if M:
- if value is None:
- if key.lower() in Morsel._flags:
- M[key] = True
+ if not morsel_seen:
+ # Invalid cookie string
+ return
+ if value is None:
+ if key.lower() in Morsel._flags:
+ parsed_items.append((TYPE_ATTRIBUTE, key, True))
else:
- M[key] = _unquote(value)
+ # Invalid cookie string
+ return
+ else:
+ parsed_items.append((TYPE_ATTRIBUTE, key, _unquote(value)))
elif value is not None:
- rval, cval = self.value_decode(value)
+ parsed_items.append((TYPE_KEYVALUE, key, self.value_decode(value)))
+ morsel_seen = True
+ else:
+ # Invalid cookie string
+ return
+
+ # The cookie string is valid, apply it.
+ M = None # current morsel
+ for tp, key, value in parsed_items:
+ if tp == TYPE_ATTRIBUTE:
+ assert M is not None
+ M[key] = value
+ else:
+ assert tp == TYPE_KEYVALUE
+ rval, cval = value
self.__set(key, rval, cval)
M = self[key]
diff --git a/Lib/test/test_http_cookies.py b/Lib/test/test_http_cookies.py
index 2b0281e..7c0e01b 100644
--- a/Lib/test/test_http_cookies.py
+++ b/Lib/test/test_http_cookies.py
@@ -141,13 +141,6 @@
self.assertEqual(C['eggs']['httponly'], 'foo')
self.assertEqual(C['eggs']['secure'], 'bar')
- def test_bad_attrs(self):
- # issue 16611: make sure we don't break backward compatibility.
- C = cookies.SimpleCookie()
- C.load('cookie=with; invalid; version; second=cookie;')
- self.assertEqual(C.output(),
- 'Set-Cookie: cookie=with\r\nSet-Cookie: second=cookie')
-
def test_extra_spaces(self):
C = cookies.SimpleCookie()
C.load('eggs = scrambled ; secure ; path = bar ; foo=foo ')
@@ -182,7 +175,10 @@
def test_invalid_cookies(self):
# Accepting these could be a security issue
C = cookies.SimpleCookie()
- for s in (']foo=x', '[foo=x', 'blah]foo=x', 'blah[foo=x'):
+ for s in (']foo=x', '[foo=x', 'blah]foo=x', 'blah[foo=x',
+ 'Set-Cookie: foo=bar', 'Set-Cookie: foo',
+ 'foo=bar; baz', 'baz; foo=bar',
+ 'secure;foo=bar', 'Version=1;foo=bar'):
C.load(s)
self.assertEqual(dict(C), {})
self.assertEqual(C.output(), '')