RomanNum3ral
/
scrapy
mirror of https://github.com/scrapy/scrapy.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

* Ignore cookies with a public suffix as domain unless it matches the request domain

* Fix the merge of 1.8.2 release notes

* Re-apply removal of tldextract restriction
This commit is contained in:
Adrián Chaves 2022-03-01 12:38:19 +01:00 committed by GitHub
parent 8ce01b3b76
commit e865c4430e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 226 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
docs/news.rst
View File

@ -52,6 +52,18 @@ Security bug fixes
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 from a
controlled domain into your cookiejar that could be sent to other domains
not controlled by the attacker. 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
Modified requirements
~~~~~~~~~~~~~~~~~~~~~
@ -1875,6 +1887,7 @@ affect subclasses:
(:issue:`3884`)
.. _release-1.8.2:
Scrapy 1.8.2 (2022-03-01)
@ -1907,6 +1920,17 @@ 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:

34
scrapy/downloadermiddlewares/cookies.py
View File

@ -1,15 +1,26 @@
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_unicode
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:
"""This middleware enables working with sites that need cookies"""
@ -23,14 +34,29 @@ class CookiesMiddleware:
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
cookiejarkey = request.meta.get("cookiejar")
jar = self.jars[cookiejarkey]
for cookie in self._get_request_cookies(jar, request):
jar.set_cookie_if_ok(cookie, request)
cookies = self._get_request_cookies(jar, request)
self._process_cookies(cookies, jar=jar, request=request)
# set Cookie header
request.headers.pop('Cookie', None)
@ -44,7 +70,9 @@ class CookiesMiddleware:
# 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

1
setup.py
View File

@ -32,6 +32,7 @@ install_requires = [
'protego>=0.1.15',
'itemadapter>=0.1.0',
'setuptools',
'tldextract',
]
extras_require = {}
cpython_dependencies = [

170
tests/test_downloadermiddleware_cookies.py
View File

@ -15,6 +15,48 @@ from scrapy.utils.python import to_bytes
from scrapy.utils.test import get_crawler
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):
@ -523,3 +565,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,
)