Skip to content
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.

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

[3.5] bpo-39503: CVE-2020-8492: Fix AbstractBasicAuthHandler (GH-18284) #19305

Merged
merged 2 commits into from Jun 20, 2020

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Apr 2, 2020

The AbstractBasicAuthHandler class of the urllib.request module uses
an inefficient regular expression which can be exploited by an
attacker to cause a denial of service. Fix the regex to prevent the
catastrophic backtracking. Vulnerability reported by Ben Caller
and Matt Schwager.

AbstractBasicAuthHandler of urllib.request now parses all
WWW-Authenticate HTTP headers and accepts multiple challenges per
header: use the realm of the first Basic challenge.

Co-Authored-By: Serhiy Storchaka storchaka@gmail.com
(cherry picked from commit 0b297d4)

https://bugs.python.org/issue39503

@vstinner
Copy link
Member Author

@vstinner vstinner commented Apr 2, 2020

For the backport, I replaced f-string with str.format() in tests.

The AbstractBasicAuthHandler class of the urllib.request module uses
an inefficient regular expression which can be exploited by an
attacker to cause a denial of service. Fix the regex to prevent the
catastrophic backtracking. Vulnerability reported by Ben Caller
and Matt Schwager.

AbstractBasicAuthHandler of urllib.request now parses all
WWW-Authenticate HTTP headers and accepts multiple challenges per
header: use the realm of the first Basic challenge.

Co-Authored-By: Serhiy Storchaka <storchaka@gmail.com>
(cherry picked from commit 0b297d4)
@vstinner vstinner force-pushed the urllib_basic_auth_regex35 branch from 134d58e to e213d74 Compare Apr 3, 2020
@vstinner
Copy link
Member Author

@vstinner vstinner commented Apr 3, 2020

PR rebased on top of commit ed07522 (remove "Codecov patch" job from the CI).

@vstinner
Copy link
Member Author

@vstinner vstinner commented Apr 3, 2020

cc @larryhastings: the CI is now green.

@vstinner
Copy link
Member Author

@vstinner vstinner commented Apr 6, 2020

@larryhastings: This change is backward incompatible. If a HTTP header contains two Basic challenges, now the first one is used, whereas previously the last one was chosen. IMO the new behavior is more correct than the old one, but I prefer to warn you ;-)

@vstinner
Copy link
Member Author

@vstinner vstinner commented May 15, 2020

Ping @larryhastings: Would you mind to merge this backport to 3.5 of a security fix?

larryhastings
larryhastings previously requested changes Jun 12, 2020
Lib/urllib/request.py Show resolved Hide resolved
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jun 12, 2020

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@larryhastings larryhastings dismissed their stale review Jun 20, 2020

I was mistaken, and no changes are needed.

@larryhastings larryhastings merged commit 37fe316 into python:3.5 Jun 20, 2020
4 checks passed
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jun 20, 2020

@larryhastings: Please replace # with GH- in the commit message next time. Thanks!

@vstinner vstinner deleted the urllib_basic_auth_regex35 branch Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants