From e2e72567a1c94c548868f6ee5329363e6036057a Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 25 Feb 2022 13:31:03 +0200 Subject: [PATCH] bpo-46756: Fix authorization check in urllib.request (GH-31353) Fix a bug in urllib.request.HTTPPasswordMgr.find_user_password() and urllib.request.HTTPPasswordMgrWithPriorAuth.is_authenticated() which allowed to bypass authorization. For example, access to URI "example.org/foobar" was allowed if the user was authorized for URI "example.org/foo". --- Lib/test/test_urllib2.py | 25 ++++++++++++++++--- Lib/urllib/request.py | 8 +++--- .../2022-02-15-11-57-53.bpo-46756.AigSPi.rst | 5 ++++ 3 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-02-15-11-57-53.bpo-46756.AigSPi.rst diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 2c93fcada43..a4772a5b1d9 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -164,7 +164,6 @@ class RequestHdrsTests(unittest.TestCase): self.assertEqual(find_user_pass("Some Realm", "http://example.com/spam"), ('joe', 'password')) - self.assertEqual(find_user_pass("Some Realm", "http://example.com/spam/spam"), ('joe', 'password')) @@ -173,12 +172,29 @@ class RequestHdrsTests(unittest.TestCase): add("c", "http://example.com/foo", "foo", "ni") add("c", "http://example.com/bar", "bar", "nini") + add("c", "http://example.com/foo/bar", "foobar", "nibar") self.assertEqual(find_user_pass("c", "http://example.com/foo"), ('foo', 'ni')) - self.assertEqual(find_user_pass("c", "http://example.com/bar"), ('bar', 'nini')) + self.assertEqual(find_user_pass("c", "http://example.com/foo/"), + ('foo', 'ni')) + self.assertEqual(find_user_pass("c", "http://example.com/foo/bar"), + ('foo', 'ni')) + self.assertEqual(find_user_pass("c", "http://example.com/foo/baz"), + ('foo', 'ni')) + self.assertEqual(find_user_pass("c", "http://example.com/foobar"), + (None, None)) + + add("c", "http://example.com/baz/", "baz", "ninini") + + self.assertEqual(find_user_pass("c", "http://example.com/baz"), + (None, None)) + self.assertEqual(find_user_pass("c", "http://example.com/baz/"), + ('baz', 'ninini')) + self.assertEqual(find_user_pass("c", "http://example.com/baz/bar"), + ('baz', 'ninini')) # For the same path, newer password should be considered. @@ -1658,8 +1674,9 @@ class HandlerTests(unittest.TestCase): auth_prior_handler.add_password( None, request_url, user, password, is_authenticated=True) - is_auth = pwd_manager.is_authenticated(request_url) - self.assertTrue(is_auth) + self.assertTrue(pwd_manager.is_authenticated(request_url)) + self.assertTrue(pwd_manager.is_authenticated(request_url + '/nested')) + self.assertFalse(pwd_manager.is_authenticated(request_url + 'plain')) opener = OpenerDirector() opener.add_handler(auth_prior_handler) diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py index fd6fc36aee0..02f96265a89 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -889,10 +889,10 @@ class HTTPPasswordMgr: return True if base[0] != test[0]: return False - common = posixpath.commonprefix((base[1], test[1])) - if len(common) == len(base[1]): - return True - return False + prefix = base[1] + if prefix[-1:] != '/': + prefix += '/' + return test[1].startswith(prefix) class HTTPPasswordMgrWithDefaultRealm(HTTPPasswordMgr): diff --git a/Misc/NEWS.d/next/Library/2022-02-15-11-57-53.bpo-46756.AigSPi.rst b/Misc/NEWS.d/next/Library/2022-02-15-11-57-53.bpo-46756.AigSPi.rst new file mode 100644 index 00000000000..1660640c5d3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-02-15-11-57-53.bpo-46756.AigSPi.rst @@ -0,0 +1,5 @@ +Fix a bug in :meth:`urllib.request.HTTPPasswordMgr.find_user_password` and +:meth:`urllib.request.HTTPPasswordMgrWithPriorAuth.is_authenticated` which +allowed to bypass authorization. For example, access to URI +``example.org/foobar`` was allowed if the user was authorized for URI +``example.org/foo``.