GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||
GeezerVison is showing github.com — gh-102153: fix CVE-2023-24329 by xiaoge1001 · Pull Request #102470 · python/cpython Conversation
|
|
Before applying #102470: >>> from urllib.parse import urlparse, urlsplit >>> urlparse(" https://example.com") ParseResult(scheme='', netloc='', path=' https://example.com', params='', query='', fragment='') After applying #102470: >>> from urllib.parse import urlparse, urlsplit >>> urlparse(" https://example.com") ParseResult(scheme='https', netloc='example.com', path='', params='', query='', fragment='') |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
BTW, this solution is mentioned at the end of the issue finder's article, see: https://pointernull.com/security/python-url-parse-problem.html for more details. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yes, I read this article, too. About the leading blank problem, I can solve it using lstrip() function even without reading this article. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
As described by @illia-v, I think it is necessary to remove the leading blanks from the URL in urllib.parse. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Right, but a few characters not covered by simple |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Thanks CharlieZhao95
|
i think that the PR of @xiaoge1001 is a good idea, could you review the PR, my brother @gpshead |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hello, is there anyone concerned about this problem? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thank you for the PRs, someone will review them and let's be patient in the meantime. See also #102153 (comment) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
#102508 is more along the lines of what we'll want to consider. I'm closing this one in favor of that. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@gpshead I'm sorry to bother you, but I don't know urllib very well. Can you answer my question? If I use this solution (https://github.com/python/cpython/pull/102470) to fix CVE when I don't think about compatibility, can the CVE be completely fixed? Will other problems be introduced? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
We aren't currently confident that we can safely backport a fix to older Python versions as something likely depends on some of the behavior around spaces. That said, this solution likely does work for a lot of code. It is a reasonable recommendation (as I think the blog post suggested?) for people to call .lstrip() themselves on their url before passing it to urlparse from their own code rather than assuming they're running on top of a Python without the issue. The other PR I closed this in favor of will have potentially even more side effects that could impact other uses of urlparse as it removes a wider/different set of characters than If you do go with this or any of the PRs in your own patched Python interpreter, please let us know how that went. Details like the relative size of your overall Python code base using your patched interpreter was and how many, if any, issues around the behavior change in the API came up after some period of time testing and deploying things with it. Those are all good data to have to inform decisions. (I'll be trying some testing of that nature myself) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
@gpshead yes, it may have more side effects. But it removes the characters only at the beginning and the end of a URL string (and a scheme if one is passed separately.) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
I plan to add constraints to fix CVE: "The first character of the URL-scheme string must be one ASCII alpha." Then I initiated a check on users for this constraint for two weeks, and did not receive any feedback on the impact. I think the impact of fixing the CVE problem should be small. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.