bpo-43075: Fix ReDoS in request #24391
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). CLA MissingOur records indicate the following people have not signed the CLA: For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day We also demand... A SHRUBBERY! You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
Hi @yetingli - I am not sure, but do you think if it makes sense to demonstrate (better /linear) performance with request parsing of the Header in the urllib testcase with this change? Something that was demonstrated in https://bugs.python.org/file49778/redos_python.py - Not necessarily using pyperf. As I said, I am not sure what that kind of test will be. |
|
I concur with @orsenthil. Just add a code which 8000 takes less than a second with a fix, and more than a hour without it. It should be enough to notice regression if we break that code. Say a million of commas. |
| @@ -0,0 +1 @@ | |||
| Fix ReDoS in request | |||
There was a problem hiding this comment.
Please be more informative. If you are Python user reading changelog, what it would say to you? What request? What type of ReDoS? Does it affect your code if it does so and so, or you can ignore it?
|
This PR is stale because it has been open for 30 days with no activity. |
|
Hi, I saw the mail in Core Mentorship. I'd love to work on this! Thanks @orsenthil |
|
This PR is stale because it has been open for 30 days with no activity. |
There was a problem hiding this comment.
Can you please update the NEWS entry, as @serhiy-storchaka requested?
For the regex, we can use your fix. It's good enough. But I'm curious if we cannot be stricter about it.
| @@ -939,7 +939,7 @@ class AbstractBasicAuthHandler: | |||
| # (single quotes are a violation of the RFC, but appear in the wild) | |||
| rx = re.compile('(?:^|,)' # start of the string or ',' | |||
| '[ \t]*' # optional whitespaces | |||
| '([^ \t]+)' # scheme like "Basic" | |||
| '([^ \t,]+)' # scheme like "Basic" | |||
There was a problem hiding this comment.
This fix is correct, but can't we be more strict to parse the scheme? Do we really want to accept non-ASCII characters in the scheme? Or Unicode spaces? HTTP is like very conservative and I expect the scheme to be only made of ASCII letters maybe with "_" and "-" characters. Is there a RFC giving the grammar for the HTTP Basic scheme?
Thank you for your reminder. Sorry, I was not familiar with the specification of PR submission before, and now I have updated the NEWS entry. |
Misc/NEWS.d/next/Security/2021-01-31-05-28-14.bpo-43075.DoAXqO.rst
Outdated
Show resolved
Hide resolved
….rst Co-authored-by: Victor Stinner <vstinner@python.org>
| @@ -0,0 +1 @@ | |||
| Fix Regular Expression Denial of Service (ReDoS) vulnerability in :class:`urllib.request.AbstractBasicAuthHandler`. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client server and needs remote attackers to control the HTTP server. | |||
There was a problem hiding this comment.
"This ReDoS issue is on the client server"
Do you mean "client side"?
There was a problem hiding this comment.
Yes, it's the “client side”. I wrote it based on your reviews:joy:, and I'll change it right away.
Please explain that the issue is on the client server, the attacker needs to control the HTTP server.
|
GH-25247 is a backport of this pull request to the 3.9 branch. |
Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn>
|
GH-25248 is a backport of this pull request to the 3.8 branch. |
Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn>
|
GH-25249 is a backport of this pull request to the 3.7 branch. |
|
GH-25250 is a backport of this pull request to the 3.6 branch. |
Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn>
Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn>
Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn>
…H-25247) Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn> Co-authored-by: Yeting Li <liyt@ios.ac.cn>
Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn> (backported to Python 2.7 by Michał Górny)
…25249) Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn>
…H-25250) Fix Regular Expression Denial of Service (ReDoS) vulnerability in urllib.request.AbstractBasicAuthHandler. The ReDoS-vulnerable regex has quadratic worst-case complexity and it allows cause a denial of service when identifying crafted invalid RFCs. This ReDoS issue is on the client side and needs remote attackers to control the HTTP server. (cherry picked from commit 7215d1a) Co-authored-by: Yeting Li <liyt@ios.ac.cn>
|
CVE-2021-3733 has now been assigned to this security bug. |
https://bugs.python.org/issue43075