[issue35647] Cookie path check returns incorrect results

[STINNER Victor]

STINNER Victor  added the comment:

Well done Karthikeyan Singaravelan! I planned to review your 2.7 change,
but I didn't manage to find time for that. So thanks Serhiy for merging it!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan  added the comment:

Closing this as resolved since the fix was merged to all branches. Thank you 
all.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset ee15aa2b8501718cb77e339381f72409a416f801 by Serhiy Storchaka 
(Xtreak) in branch '2.7':
[2.7] bpo-35647: Fix path check in cookiejar. (GH-11436) (GH-13427)
https://github.com/python/cpython/commit/ee15aa2b8501718cb77e339381f72409a416f801


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan  added the comment:

> Python 2.7 is still affected, right? Is there someone interested to backport 
> the fix?

PR 13427 fixes the issue in 2.7 :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[STINNER Victor]

STINNER Victor  added the comment:

Python 2.7 is still affected, right? Is there someone interested to backport 
the fix?

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
pull_requests: +13337

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Ned Deily]

Change by Ned Deily :


--
Removed message: https://bugs.python.org/msg342091

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Ned Deily]

Ned Deily  added the comment:


New changeset e260f092cd0d8975c777e73ca6fb549d59b5d452 by larryhastings 
(Xtreak) in branch '3.4':
bpo-35647: Fix path check in cookiejar (#11436) (#12278)
https://github.com/python/cpython/commit/e260f092cd0d8975c777e73ca6fb549d59b5d452


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Larry Hastings]

Larry Hastings  added the comment:


New changeset 382981b25092b5e9285f1e4894142af1e8f2ca86 by larryhastings 
(Xtreak) in branch '3.5':
bpo-35647: Fix path check in cookiejar (#11436) (#12277)
https://github.com/python/cpython/commit/382981b25092b5e9285f1e4894142af1e8f2ca86


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Larry Hastings]

Larry Hastings  added the comment:


New changeset e260f092cd0d8975c777e73ca6fb549d59b5d452 by larryhastings 
(Xtreak) in branch '3.4':
bpo-35647: Fix path check in cookiejar (#11436) (#12278)
https://github.com/python/cpython/commit/e260f092cd0d8975c777e73ca6fb549d59b5d452


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Senthil Kumaran]

Senthil Kumaran  added the comment:

Got it, Larry. I wanted to track it and make sure you don't miss it. Looks like 
we have the PR from @xtreak for 3.4 and 3.5 branches.
Thank you!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Larry Hastings]

Larry Hastings  added the comment:

You should not assign bugs to the RM who will merge the PR.

--
assignee: larry -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Senthil Kumaran]

Change by Senthil Kumaran :


--
assignee:  -> larry

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Ned Deily]

Ned Deily  added the comment:


New changeset 5565b1db6f37f244890369e0d68a3e906aca28b9 by Ned Deily (Miss 
Islington (bot)) in branch '3.6':
bpo-35647: Fix path check in cookiejar (GH-11436) (GH-12268)
https://github.com/python/cpython/commit/5565b1db6f37f244890369e0d68a3e906aca28b9


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
versions: +Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
pull_requests: +12258

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
pull_requests: +12257

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Larry Hastings]

Larry Hastings  added the comment:

Yes, I'd like backports to 3.5 and 3.4.  Note that I tag and release 3.4.10 
final this weekend, which will be the final release ever of 3.4, so if it can't 
be ready for merging before then, you might as well skip the 3.4 PR.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan  added the comment:

The backport to 3.5 might require manual work since I used f-strings for tests 
that are not present in 3.5 and below. 2.7 is also affected and as I backported 
the tests and cookie set with path=/foo is sent on request to /foobad/foo . The 
module is present under Lib/cookielb.py and might also require a different 
backport. Since this applies RFC 6265 definition that is more stricter and 
concrete than RFC 2965 I am not sure if this might break someone's code though 
there is a bug in the paths to which the cookie is sent. I am adding Larry and 
Benjamin who can take a call on backport and if a backport is needed I will be 
happy to open respective PRs.

The code in 2.7 also performs the same prefix match at 
https://github.com/python/cpython/blob/55438d713978a1913ef12c8a801848626228aad6/Lib/cookielib.py#L1182
 that was fixed as per RFC 6265 .

def path_return_ok(self, path, request):
_debug("- checking cookie path=%s", path)
req_path = request_path(request)
if not req_path.startswith(path):
_debug("  %s does not path-match %s", req_path, path)
return False
return True


$ ./python.exe
Python 2.7.16+ (remotes/upstream/2.7-dirty:55438d7139, Mar 10 2019, 23:35:15)
[GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>>
$ ./python.exe -m unittest -v 
test.test_cookielib.CookieTests.test_path_prefix_match
test_path_prefix_match (test.test_cookielib.CookieTests) ... FAIL

==
FAIL: test_path_prefix_match (test.test_cookielib.CookieTests)
--
Traceback (most recent call last):
  File 
"/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/test/test_cookielib.py",
 line 673, in test_path_prefix_match
self.assertNotIn('spam=eggs', h, "cookie set for {0}".format(path))
AssertionError: cookie set for /foobad/foo

--
Ran 1 test in 0.010s

FAILED (failures=1)

--
nosy: +benjamin.peterson, larry
versions: +Python 2.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[miss-islington]

miss-islington  added the comment:


New changeset 97c7d78fda49e03fc773c171ce0c736d02bb73f5 by Miss Islington (bot) 
in branch '3.7':
bpo-35647: Fix path check in cookiejar (GH-11436)
https://github.com/python/cpython/commit/97c7d78fda49e03fc773c171ce0c736d02bb73f5


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Senthil Kumaran]

Change by Senthil Kumaran :


--
versions: +Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[miss-islington]

Change by miss-islington :


--
pull_requests: +12252

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Senthil Kumaran]

Senthil Kumaran  added the comment:


New changeset 0e1f1f01058bd4a9b98cfe443214adecc019a38c by Senthil Kumaran 
(Xtreak) in branch 'master':
bpo-35647: Fix path check in cookiejar (#11436)
https://github.com/python/cpython/commit/0e1f1f01058bd4a9b98cfe443214adecc019a38c


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[miss-islington]

Change by miss-islington :


--
pull_requests: +12251

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
keywords: +patch, patch, patch
pull_requests: +10868, 10869, 10870
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
keywords: +patch, patch
pull_requests: +10868, 10869
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
keywords: +patch
pull_requests: +10868
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue35647] Cookie path check returns incorrect results

[Karthikeyan Singaravelan]

New submission from Karthikeyan Singaravelan :

I came across the issue during https://bugs.python.org/issue35121#msg332583 . I 
think this can be dealt as a separate issue not blocking the original report. I 
am classifying it as security but can be reclassified as a bug fix given the 
section on weak confidentiality in RFC 6265. I have a fix implemented at 
https://github.com/tirkarthi/cpython/tree/bpo35121-cookie-path.

Report : 

I have come across another behavior change between path checks while using the 
cookie jar implementation available in Python. This is related to incorrect 
cookie validation but with respect to path. I observed the following difference 
: 

1. Make a request to "/" that sets a cookie with "path=/any"
2. Make a request to "/any" and the set cookie is passed since the path matches
3. Make a request to "/anybad" and cookie with "path=/any" is also passed too.

On using golang stdlib implementation of cookiejar the cookie "path=/any" is 
not passed when a request to "/anybad" is made. So I checked with RFC 6265 
where the path match check is defined in section-5.1.4 . RFC 6265 also 
obsoletes RFC 2965 upon which cookiejar is based I hope since original 
implementation of cookiejar is from 2004 and RFC 6265 was standardized later. 
So I think it's good to enforce RFC 6265 since RFC 2965 is obsolete at least in 
Python 3.8 unless this is considered as a security issue. I think this is a 
security issue. The current implementation can potentially cause cookies to be 
sent to incorrect paths in the same domain that share the same prefix. This is 
a behavior change with more strict checks but I could see no tests failing with 
RFC 6265 implementation too.

RFC 2965 also gives a loose definition of path-match without mentioning about / 
check in the paths based on which Python implementation is based as a simple 
prefix match.

> For two strings that represent paths, P1 and P2, P1 path-matches P2
> if P2 is a prefix of P1 (including the case where P1 and P2 string-
> compare equal).  Thus, the string /tec/waldo path-matches /tec.

RFC 6265 path-match definition : 
https://tools.ietf.org/html/rfc6265#section-5.1.4

   A request-path path-matches a given cookie-path if at least one of
   the following conditions holds:

   o  The cookie-path and the request-path are identical.

   o  The cookie-path is a prefix of the request-path, and the last
  character of the cookie-path is %x2F ("/").

   o  The cookie-path is a prefix of the request-path, and the first
  character of the request-path that is not included in the cookie-
  path is a %x2F ("/") character.

The current implementation in cookiejar is as below : 

def path_return_ok(self, path, request):
_debug("- checking cookie path=%s", path)
req_path = request_path(request)
if not req_path.startswith(path):
_debug("  %s does not path-match %s", req_path, path)
return False
return True

Translating the RFC 6265 steps (a literal translation of go implementation) 
would have something like below and no tests fail on master. So the python 
implementation goes in line with the RFC not passing cookies of "path=/any" to 
/anybody

def path_return_ok(self, path, request):
req_path = request_path(request)
if req_path == path:
return True
elif req_path.startswith(path) and ((path.endswith("/") or 
req_path[len(path)] == "/")):
return True

return False


The golang implementation is as below which is a literal translation of RFC 
6265 steps at 
https://github.com/golang/go/blob/50bd1c4d4eb4fac8ddeb5f063c099daccfb71b26/src/net/http/cookiejar/jar.go#L130

// pathMatch implements "path-match" according to RFC 6265 section 5.1.4.
func (e *entry) pathMatch(requestPath string) bool {
if requestPath == e.Path {
return true
}
if strings.HasPrefix(requestPath, e.Path) {
if e.Path[len(e.Path)-1] == '/' {
return true // The "/any/" matches "/any/path" case.
} else if requestPath[len(e.Path)] == '/' {
return true // The "/any" matches "/any/path" case.
}
}
return false
}


RFC 6265 on weak confidentiality 
(https://tools.ietf.org/html/rfc6265#section-8.5)

   Cookies do not always provide isolation by path.  Although the
   network-level protocol does not send cookies stored for one path to
   another, some user agents expose cookies via non-HTTP APIs, such as
   HTML's document.cookie API.  Because some of these user agents (e.g.,
   web browsers) do not isolate resources received from different paths,
   a resource retrieved from one path might be able to access cookies
   stored for another path.

--
components: Library (Lib)
messages: 332919
nosy: martin.panter, ned.deily, orsenthil, serhiy.storchaka, xtreak
priority: normal
severity: normal
status: open
title: Cookie path check returns incorrect results