Merge pull request from GHSA-mfjm-vh54-3f96

This commit is contained in:
Adrián Chaves 2022-03-01 12:38:19 +01:00 committed by GitHub
parent 290e2a2726
commit 91706f58b7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 213 additions and 3 deletions

View File

@ -38,6 +38,18 @@ Scrapy 1.8.2 (2022-03-01)
your cookies. See the documentation of the
:class:`~scrapy.http.Request` class for more information.
- When the domain of a cookie, either received in the ``Set-Cookie`` header
of a response or defined in a :class:`~scrapy.http.Request` object, is set
to a `public suffix <https://publicsuffix.org/>`_, the cookie is now
ignored unless the cookie domain is the same as the request domain.
The old behavior could be exploited by an attacker to inject cookies into
your requests to some other domains. Please, see the `mfjm-vh54-3f96
security advisory`_ for more information.
.. _mfjm-vh54-3f96 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-mfjm-vh54-3f96
.. _release-1.8.1:
Scrapy 1.8.1 (2021-10-05)

View File

@ -3,14 +3,25 @@ import six
import logging
from collections import defaultdict
from tldextract import TLDExtract
from scrapy.exceptions import NotConfigured
from scrapy.http import Response
from scrapy.http.cookies import CookieJar
from scrapy.utils.httpobj import urlparse_cached
from scrapy.utils.python import to_native_str
logger = logging.getLogger(__name__)
_split_domain = TLDExtract(include_psl_private_domains=True)
def _is_public_domain(domain):
parts = _split_domain(domain)
return not parts.domain
class CookiesMiddleware(object):
"""This middleware enables working with sites that need cookies"""
@ -24,6 +35,21 @@ class CookiesMiddleware(object):
raise NotConfigured
return cls(crawler.settings.getbool('COOKIES_DEBUG'))
def _process_cookies(self, cookies, *, jar, request):
for cookie in cookies:
cookie_domain = cookie.domain
if cookie_domain.startswith('.'):
cookie_domain = cookie_domain[1:]
request_domain = urlparse_cached(request).hostname.lower()
if cookie_domain and _is_public_domain(cookie_domain):
if cookie_domain != request_domain:
continue
cookie.domain = request_domain
jar.set_cookie_if_ok(cookie, request)
def process_request(self, request, spider):
if request.meta.get('dont_merge_cookies', False):
return
@ -31,8 +57,7 @@ class CookiesMiddleware(object):
cookiejarkey = request.meta.get("cookiejar")
jar = self.jars[cookiejarkey]
cookies = self._get_request_cookies(jar, request)
for cookie in cookies:
jar.set_cookie_if_ok(cookie, request)
self._process_cookies(cookies, jar=jar, request=request)
# set Cookie header
request.headers.pop('Cookie', None)
@ -46,7 +71,9 @@ class CookiesMiddleware(object):
# extract cookies from Set-Cookie and drop invalid/expired cookies
cookiejarkey = request.meta.get("cookiejar")
jar = self.jars[cookiejarkey]
jar.extract_cookies(response, request)
cookies = jar.make_cookies(response, request)
self._process_cookies(cookies, jar=jar, request=request)
self._debug_set_cookie(response, spider)
return response

View File

@ -79,6 +79,7 @@ setup(
'w3lib>=1.17.0',
'zope.interface>=4.1.3',
'protego>=0.1.15',
'tldextract',
],
extras_require=extras_require,
)

View File

@ -12,6 +12,48 @@ from scrapy.exceptions import NotConfigured
from scrapy.downloadermiddlewares.cookies import CookiesMiddleware
def _cookie_to_set_cookie_value(cookie):
"""Given a cookie defined as a dictionary with name and value keys, and
optional path and domain keys, return the equivalent string that can be
associated to a ``Set-Cookie`` header."""
decoded = {}
for key in ("name", "value", "path", "domain"):
if cookie.get(key) is None:
if key in ("name", "value"):
return
continue
if isinstance(cookie[key], (bool, float, int, str)):
decoded[key] = str(cookie[key])
else:
try:
decoded[key] = cookie[key].decode("utf8")
except UnicodeDecodeError:
decoded[key] = cookie[key].decode("latin1", errors="replace")
cookie_str = f"{decoded.pop('name')}={decoded.pop('value')}"
for key, value in decoded.items(): # path, domain
cookie_str += f"; {key.capitalize()}={value}"
return cookie_str
def _cookies_to_set_cookie_list(cookies):
"""Given a group of cookie defined either as a dictionary or as a list of
dictionaries (i.e. in a format supported by the cookies parameter of
Request), return the equivalen list of strings that can be associated to a
``Set-Cookie`` header."""
if not cookies:
return []
if isinstance(cookies, dict):
cookies = ({"name": k, "value": v} for k, v in cookies.items())
return filter(
None,
(
_cookie_to_set_cookie_value(cookie)
for cookie in cookies
)
)
class CookiesMiddlewareTest(TestCase):
def assertCookieValEqual(self, first, second, msg=None):
@ -384,3 +426,131 @@ class CookiesMiddlewareTest(TestCase):
{'url': 'https://example.com', 'status': 302},
cookies2=False,
)
def _test_user_set_cookie_domain_followup(
self,
url1,
url2,
domain,
*,
cookies1,
cookies2,
):
input_cookies = [
{
'name': 'a',
'value': 'b',
'domain': domain,
}
]
request1 = Request(url1, cookies=input_cookies)
self.mw.process_request(request1, self.spider)
cookies = request1.headers.get('Cookie')
self.assertEqual(cookies, b"a=b" if cookies1 else None)
request2 = Request(url2)
self.mw.process_request(request2, self.spider)
cookies = request2.headers.get('Cookie')
self.assertEqual(cookies, b"a=b" if cookies2 else None)
def test_user_set_cookie_domain_suffix_private(self):
self._test_user_set_cookie_domain_followup(
'https://books.toscrape.com',
'https://quotes.toscrape.com',
'toscrape.com',
cookies1=True,
cookies2=True,
)
def test_user_set_cookie_domain_suffix_public_period(self):
self._test_user_set_cookie_domain_followup(
'https://foo.co.uk',
'https://bar.co.uk',
'co.uk',
cookies1=False,
cookies2=False,
)
def test_user_set_cookie_domain_suffix_public_private(self):
self._test_user_set_cookie_domain_followup(
'https://foo.blogspot.com',
'https://bar.blogspot.com',
'blogspot.com',
cookies1=False,
cookies2=False,
)
def test_user_set_cookie_domain_public_period(self):
self._test_user_set_cookie_domain_followup(
'https://co.uk',
'https://co.uk',
'co.uk',
cookies1=True,
cookies2=True,
)
def _test_server_set_cookie_domain_followup(
self,
url1,
url2,
domain,
*,
cookies,
):
request1 = Request(url1)
self.mw.process_request(request1, self.spider)
input_cookies = [
{
'name': 'a',
'value': 'b',
'domain': domain,
}
]
headers = {
'Set-Cookie': _cookies_to_set_cookie_list(input_cookies),
}
response = Response(url1, status=200, headers=headers)
self.assertEqual(
self.mw.process_response(request1, response, self.spider),
response,
)
request2 = Request(url2)
self.mw.process_request(request2, self.spider)
actual_cookies = request2.headers.get('Cookie')
self.assertEqual(actual_cookies, b"a=b" if cookies else None)
def test_server_set_cookie_domain_suffix_private(self):
self._test_server_set_cookie_domain_followup(
'https://books.toscrape.com',
'https://quotes.toscrape.com',
'toscrape.com',
cookies=True,
)
def test_server_set_cookie_domain_suffix_public_period(self):
self._test_server_set_cookie_domain_followup(
'https://foo.co.uk',
'https://bar.co.uk',
'co.uk',
cookies=False,
)
def test_server_set_cookie_domain_suffix_public_private(self):
self._test_server_set_cookie_domain_followup(
'https://foo.blogspot.com',
'https://bar.blogspot.com',
'blogspot.com',
cookies=False,
)
def test_server_set_cookie_domain_public_period(self):
self._test_server_set_cookie_domain_followup(
'https://co.uk',
'https://co.uk',
'co.uk',
cookies=True,
)