Skip to content

bpo-35121: prefix dot in domain for proper subdomain validation #10258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 10, 2019
Merged

bpo-35121: prefix dot in domain for proper subdomain validation #10258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d67d18e
Prefix domain with dot for proper subdomain validation in domain_retu…
tirkarthi Oct 31, 2018
2816aa8
Add NEWS entry
tirkarthi Oct 31, 2018
dfdc776
Prefix dot only for suffix check and add test
tirkarthi Dec 24, 2018
ecae447
Reword news entry and added extra test
tirkarthi Dec 24, 2018
b2ab4a3
Refactor if clause and fix news entry
tirkarthi Dec 24, 2018
b8e2df1
Ensure return_ok_domain does proper validation
tirkarthi Dec 25, 2018
9d1eed3
Move NEWS to security
tirkarthi Mar 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions Lib/http/cookiejar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,11 @@ def return_ok_domain(self, cookie, request):
req_host, erhn = eff_request_host(request)
domain = cookie.domain

if domain and not domain.startswith("."):
dotdomain = "." + domain
else:
dotdomain = domain

# strict check of non-domain cookies: Mozilla does this, MSIE5 doesn't
if (cookie.version == 0 and
(self.strict_ns_domain & self.DomainStrictNonDomain) and
Expand All @@ -1159,7 +1164,7 @@ def return_ok_domain(self, cookie, request):
_debug(" effective request-host name %s does not domain-match "
"RFC 2965 cookie domain %s", erhn, domain)
return False
if cookie.version == 0 and not ("."+erhn).endswith(domain):
if cookie.version == 0 and not ("."+erhn).endswith(dotdomain):
_debug(" request-host %s does not match Netscape cookie domain "
"%s", req_host, domain)
return False
Expand All @@ -1173,7 +1178,11 @@ def domain_return_ok(self, domain, request):
req_host = "."+req_host
if not erhn.startswith("."):
erhn = "."+erhn
if not (req_host.endswith(domain) or erhn.endswith(domain)):
if domain and not domain.startswith("."):
dotdomain = "." + domain
else:
dotdomain = domain
if not (req_host.endswith(dotdomain) or erhn.endswith(dotdomain)):
#_debug(" request domain %s does not match cookie domain %s",
# req_host, domain)
return False
Expand Down
30 changes: 30 additions & 0 deletions Lib/test/test_http_cookiejar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,7 @@ def test_domain_return_ok(self):
("http://foo.bar.com/", ".foo.bar.com", True),
("http://foo.bar.com/", "foo.bar.com", True),
("http://foo.bar.com/", ".bar.com", True),
("http://foo.bar.com/", "bar.com", True),
("http://foo.bar.com/", "com", True),
("http://foo.com/", "rhubarb.foo.com", False),
("http://foo.com/", ".foo.com", True),
Expand All @@ -425,6 +426,8 @@ def test_domain_return_ok(self):
("http://foo/", "foo", True),
("http://foo/", "foo.local", True),
("http://foo/", ".local", True),
("http://barfoo.com", ".foo.com", False),
("http://barfoo.com", "foo.com", False),
]:
request = urllib.request.Request(url)
r = pol.domain_return_ok(domain, request)
Expand Down Expand Up @@ -959,6 +962,33 @@ def test_domain_block(self):
c.add_cookie_header(req)
self.assertFalse(req.has_header("Cookie"))

c.clear()

pol.set_blocked_domains([])
req = urllib.request.Request("http://acme.com/")
res = FakeResponse(headers, "http://acme.com/")
cookies = c.make_cookies(res, req)
c.extract_cookies(res, req)
self.assertEqual(len(c), 1)

req = urllib.request.Request("http://acme.com/")
c.add_cookie_header(req)
self.assertTrue(req.has_header("Cookie"))

req = urllib.request.Request("http://badacme.com/")
c.add_cookie_header(req)
self.assertFalse(pol.return_ok(cookies[0], req))
self.assertFalse(req.has_header("Cookie"))

p = pol.set_blocked_domains(["acme.com"])
req = urllib.request.Request("http://acme.com/")
c.add_cookie_header(req)
self.assertFalse(req.has_header("Cookie"))

req = urllib.request.Request("http://badacme.com/")
c.add_cookie_header(req)
self.assertFalse(req.has_header("Cookie"))

def test_secure(self):
for ns in True, False:
for whitespace in " ", "":
Expand Down
4 changes: 4 additions & 0 deletions Misc/NEWS.d/next/Security/2018-10-31-15-39-17.bpo-35121.EgHv9k.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Don't send cookies of domain A without Domain attribute to domain B
when domain A is a suffix match of domain B while using a cookiejar
with :class:`http.cookiejar.DefaultCookiePolicy` policy. Patch by
Karthikeyan Singaravelan.