Skip to content

Commit 1c9701a

Browse files
bpo-46756: Fix authorization check in urllib.request (GH-31353) (GH-31572)
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". (cherry picked from commit e2e7256) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>
1 parent eb6c840 commit 1c9701a

File tree

3 files changed

+30
-8
lines changed

3 files changed

+30
-8
lines changed

Lib/test/test_urllib2.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,6 @@ def test_password_manager(self):
158158
self.assertEqual(find_user_pass("Some Realm",
159159
"http://example.com/spam"),
160160
('joe', 'password'))
161-
162161
self.assertEqual(find_user_pass("Some Realm",
163162
"http://example.com/spam/spam"),
164163
('joe', 'password'))
@@ -167,12 +166,29 @@ def test_password_manager(self):
167166

168167
add("c", "http://example.com/foo", "foo", "ni")
169168
add("c", "http://example.com/bar", "bar", "nini")
169+
add("c", "http://example.com/foo/bar", "foobar", "nibar")
170170

171171
self.assertEqual(find_user_pass("c", "http://example.com/foo"),
172172
('foo', 'ni'))
173-
174173
self.assertEqual(find_user_pass("c", "http://example.com/bar"),
175174
('bar', 'nini'))
175+
self.assertEqual(find_user_pass("c", "http://example.com/foo/"),
176+
('foo', 'ni'))
177+
self.assertEqual(find_user_pass("c", "http://example.com/foo/bar"),
178+
('foo', 'ni'))
179+
self.assertEqual(find_user_pass("c", "http://example.com/foo/baz"),
180+
('foo', 'ni'))
181+
self.assertEqual(find_user_pass("c", "http://example.com/foobar"),
182+
(None, None))
183+
184+
add("c", "http://example.com/baz/", "baz", "ninini")
185+
186+
self.assertEqual(find_user_pass("c", "http://example.com/baz"),
187+
(None, None))
188+
self.assertEqual(find_user_pass("c", "http://example.com/baz/"),
189+
('baz', 'ninini'))
190+
self.assertEqual(find_user_pass("c", "http://example.com/baz/bar"),
191+
('baz', 'ninini'))
176192

177193
# For the same path, newer password should be considered.
178194

@@ -1653,8 +1669,9 @@ def test_basic_prior_auth_auto_send(self):
16531669
auth_prior_handler.add_password(
16541670
None, request_url, user, password, is_authenticated=True)
16551671

1656-
is_auth = pwd_manager.is_authenticated(request_url)
1657-
self.assertTrue(is_auth)
1672+
self.assertTrue(pwd_manager.is_authenticated(request_url))
1673+
self.assertTrue(pwd_manager.is_authenticated(request_url + '/nested'))
1674+
self.assertFalse(pwd_manager.is_authenticated(request_url + 'plain'))
16581675

16591676
opener = OpenerDirector()
16601677
opener.add_handler(auth_prior_handler)

Lib/urllib/request.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -895,10 +895,10 @@ def is_suburi(self, base, test):
895895
return True
896896
if base[0] != test[0]:
897897
return False
898-
common = posixpath.commonprefix((base[1], test[1]))
899-
if len(common) == len(base[1]):
900-
return True
901-
return False
898+
prefix = base[1]
899+
if prefix[-1:] != '/':
900+
prefix += '/'
901+
return test[1].startswith(prefix)
902902

903903

904904
class HTTPPasswordMgrWithDefaultRealm(HTTPPasswordMgr):
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix a bug in :meth:`urllib.request.HTTPPasswordMgr.find_user_password` and
2+
:meth:`urllib.request.HTTPPasswordMgrWithPriorAuth.is_authenticated` which
3+
allowed to bypass authorization. For example, access to URI
4+
``example.org/foobar`` was allowed if the user was authorized for URI
5+
``example.org/foo``.

0 commit comments

Comments
 (0)