bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. #25595

orsenthil · 2021-04-25T14:49:58Z

bpo-43882: Strip ascii newline and tabs from the url input, following WHATWG specification

Presence newline or tab characters in URL allowed attackers to write scripts in URL, hijack the web-server.

Following the controlling specification for URLs defined by WHATWG urllib.parse strips ASCII newline and tabs from the url, preventing such attacks.

https://bugs.python.org/issue43882

…ne and tabs.

gpshead

Some code suggestions made.

Lib/test/test_urlparse.py

Lib/urllib/parse.py

Doc/library/urllib.parse.rst

bedevere-bot · 2021-04-28T07:28:53Z

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

Lib/urllib/parse.py

Doc/library/urllib.parse.rst

Misc/NEWS.d/next/Security/2021-04-25-07-46-37.bpo-43882.Jpwx85.rst

Doc/library/urllib.parse.rst

Lib/urllib/parse.py

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

….rst Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…3882

miss-islington · 2021-04-29T17:16:54Z

Thanks @orsenthil for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8, 3.9.
🐍🍒⛏🤖

…e and tabs. (pythonGH-25595) * issue43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 76cd81d) Co-authored-by: Senthil Kumaran <senthil@uthcode.com>

bedevere-bot · 2021-04-29T17:17:16Z

GH-25725 is a backport of this pull request to the 3.9 branch.

bedevere-bot · 2021-04-29T17:17:22Z

GH-25726 is a backport of this pull request to the 3.8 branch.

…e and tabs. (pythonGH-25595) * issue43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 76cd81d) Co-authored-by: Senthil Kumaran <senthil@uthcode.com>

bedevere-bot · 2021-04-29T17:17:28Z

GH-25727 is a backport of this pull request to the 3.7 branch.

…e and tabs. (pythonGH-25595) * issue43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 76cd81d) Co-authored-by: Senthil Kumaran <senthil@uthcode.com>

bedevere-bot · 2021-04-29T17:17:35Z

GH-25728 is a backport of this pull request to the 3.6 branch.

…newline and tabs. (GH-25595) (GH-25725) * bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. (GH-25595) Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 76cd81d) Co-authored-by: Senthil Kumaran <skumaran@gatech.edu>

…e and tabs. (pythonGH-25595) * issue43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 76cd81d) Co-authored-by: Senthil Kumaran <senthil@uthcode.com>

…e and tabs. (pythonGH-25595) * issue43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

…newline and tabs. (pythonGH-25595) (pythonGH-25725) * bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. (pythonGH-25595) Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 76cd81d) Co-authored-by: Senthil Kumaran <skumaran@gatech.edu> (backported to Python 2.7 by Michał Górny)

…newline and tabs. (GH-25595) (#25726) Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 76cd81d) Co-authored-by: Senthil Kumaran <senthil@uthcode.com> Co-authored-by: Senthil Kumaran <skumaran@gatech.edu>

…newline and tabs. (pythonGH-25595) (pythonGH-25726) Co-authored-by: Gregory P. Smith <greg@krypto.org> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> (cherry picked from commit 76cd81d) Co-authored-by: Senthil Kumaran <senthil@uthcode.com> Co-authored-by: Senthil Kumaran <skumaran@gatech.edu> (cherry picked from commit 515a7bc) Co-authored-by: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com>

bedevere-bot · 2021-05-05T18:14:54Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86 Gentoo Installed with X 3.8 has failed when building commit 515a7bc.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/291/builds/26) and take a look at the build logs.
Check if the failure is related to this commit (515a7bc) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/291/builds/26

Failed tests:

test_socket

Failed subtests:

testSendFrame - test.test_socket.CANTest

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

410 tests OK.

1 test failed:
test_socket

12 tests skipped:
test_asdl_parser test_clinic test_devpoll test_gdb test_ioctl
test_kqueue test_msilib test_startfile test_winconsoleio
test_winreg test_winsound test_zipfile64

1 re-run test:
test_socket

Total duration: 27 min 39 sec

Click to see traceback logs

Traceback (most recent call last):
  File "/buildbot/buildarea/cpython/3.8.ware-gentoo-x86.installed/build/target/lib/python3.8/test/test_socket.py", line 1957, in testSendFrame
    self.assertEqual(addr[1], socket.AF_CAN)
IndexError: tuple index out of range

bedevere-bot · 2021-05-05T18:21:37Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86 Gentoo Non-Debug with X 3.8 has failed when building commit 515a7bc.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/324/builds/26) and take a look at the build logs.
Check if the failure is related to this commit (515a7bc) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/324/builds/26

Failed tests:

test_socket

Failed subtests:

testSendFrame - test.test_socket.CANTest

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

413 tests OK.

10 slowest tests:

test_multiprocessing_spawn: 4 min 18 sec
test_concurrent_futures: 3 min 51 sec
test_tokenize: 3 min 13 sec
test_tools: 2 min 59 sec
test_asyncio: 2 min 52 sec
test_lib2to3: 2 min 33 sec
test_multiprocessing_forkserver: 1 min 49 sec
test_multiprocessing_fork: 1 min 38 sec
test_pickle: 1 min 10 sec
test_subprocess: 1 min 2 sec

1 test failed:
test_socket

9 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_startfile
test_winconsoleio test_winreg test_winsound test_zipfile64

1 re-run test:
test_socket

Total duration: 29 min 34 sec

Click to see traceback logs

Traceback (most recent call last):
  File "/buildbot/buildarea/cpython/3.8.ware-gentoo-x86.nondebug/build/Lib/test/test_socket.py", line 1957, in testSendFrame
    self.assertEqual(addr[1], socket.AF_CAN)
IndexError: tuple index out of range

Implement the fix from the upstream python/cpython#25595

urlsplit was changed to strip ASCII newline and tab characters in python/cpython#25595. The urlsplit change breaks cob as it was relying on urlsplit to keep a trailing newline character. This patch is backward compatible. This issue was identified on Amazon Linux 2's Python distribution. https://alas.aws.amazon.com/AL2/ALAS-2022-1802.html. urlsplit behaviour before the fix. ``` Python 2.7.18 (default, Jun 10 2021, 00:11:02) [GCC 7.3.1 20180712 (Red Hat 7.3.1-13)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from urlparse import urlsplit >>> urlsplit("http://localhost/some-path\n").path '/some-path\n' >>> ``` urlsplit behaviour after the fix. ``` Python 2.7.18 (default, May 25 2022, 14:30:51) [GCC 7.3.1 20180712 (Red Hat 7.3.1-15)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from urlparse import urlsplit >>> urlsplit("http://localhost/some-path\n").path '/some-path' >>> ```

urlsplit function was changed to strip ASCII newline and tab characters in python/cpython#25595. The change breaks S3 authentication as cob was relying on urlsplit to keep a trailing newline character. This issue was identified on Amazon Linux 2 Python distribution which backported the change to Python 2. https://alas.aws.amazon.com/AL2/ALAS-2022-1802.html. urlsplit behaviour before the fix. ``` Python 2.7.18 (default, Jun 10 2021, 00:11:02) [GCC 7.3.1 20180712 (Red Hat 7.3.1-13)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from urlparse import urlsplit >>> urlsplit("http://localhost/some-path\n").path '/some-path\n' >>> ``` urlsplit behaviour after the fix. ``` Python 2.7.18 (default, May 25 2022, 14:30:51) [GCC 7.3.1 20180712 (Red Hat 7.3.1-15)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from urlparse import urlsplit >>> urlsplit("http://localhost/some-path\n").path '/some-path' >>> ```

urlsplit function was changed to strip ASCII newline and tab characters in python/cpython#25595. The change causes this plugin to not properly authenticate with S3, since a required newline character is no longer present. This issue was identified on Amazon Linux 2 Python distribution which backported the change to Python 2. https://alas.aws.amazon.com/AL2/ALAS-2022-1802.html. urlsplit behaviour before the fix. ``` Python 2.7.18 (default, Jun 10 2021, 00:11:02) [GCC 7.3.1 20180712 (Red Hat 7.3.1-13)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from urlparse import urlsplit >>> urlsplit("http://localhost/some-path\n").path '/some-path\n' >>> ``` urlsplit behaviour after the fix. ``` Python 2.7.18 (default, May 25 2022, 14:30:51) [GCC 7.3.1 20180712 (Red Hat 7.3.1-15)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from urlparse import urlsplit >>> urlsplit("http://localhost/some-path\n").path '/some-path' >>> ```

issue43882 - urllib.parse should sanitize urls containing ASCII newli…

60c9b55

…ne and tabs.

the-knights-who-say-ni added the CLA signed label Apr 25, 2021

bedevere-bot added the awaiting core review label Apr 25, 2021

orsenthil changed the title ~~issue43882 - urllib.parse should sanitize urls containing ASCII newline and tabs.~~ bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. Apr 25, 2021

orsenthil self-assigned this Apr 25, 2021

orsenthil requested review from tirkarthi, vstinner and serhiy-storchaka Apr 25, 2021

gpshead requested changes Apr 28, 2021

View changes

Lib/test/test_urlparse.py Outdated Show resolved Hide resolved

Lib/urllib/parse.py Outdated Show resolved Hide resolved

Doc/library/urllib.parse.rst Outdated Show resolved Hide resolved

Doc/library/urllib.parse.rst Outdated Show resolved Hide resolved

bedevere-bot removed the awaiting core review label Apr 28, 2021

bedevere-bot added the awaiting changes label Apr 28, 2021

gpshead added type-bug An unexpected behavior, bug, or error type-security A security issue labels Apr 28, 2021

tirkarthi reviewed Apr 28, 2021

View changes

Lib/urllib/parse.py Outdated Show resolved Hide resolved

Address Code Review Comments.

ee779d6

orsenthil requested review from gpshead and tirkarthi Apr 28, 2021

orsenthil and others added 2 commits Apr 28, 2021

Give link to specification. Reference in WG page in Seealso links.

e81f32f

Wording cleanup and ReST editing for NEWS.

ae07b8f

gpshead approved these changes Apr 29, 2021

View changes

bedevere-bot added awaiting merge and removed awaiting changes labels Apr 29, 2021

serhiy-storchaka reviewed Apr 29, 2021

View changes

Doc/library/urllib.parse.rst Outdated Show resolved Hide resolved

Misc/NEWS.d/next/Security/2021-04-25-07-46-37.bpo-43882.Jpwx85.rst Outdated Show resolved Hide resolved

Doc/library/urllib.parse.rst Outdated Show resolved Hide resolved

serhiy-storchaka reviewed Apr 29, 2021

View changes

Lib/urllib/parse.py Show resolved Hide resolved

orsenthil and others added 5 commits Apr 29, 2021

Update Doc/library/urllib.parse.rst

908e744

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Update Misc/NEWS.d/next/Security/2021-04-25-07-46-37.bpo-43882.Jpwx85…

5f61b9e

….rst Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Update Doc/library/urllib.parse.rst

4afcf42

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Merge branch 'master' into issue43882

0885aff

Merge branch 'issue43882' of github.com:orsenthil/cpython into issue4…

1150995

…3882

bedevere-bot removed the awaiting merge label Apr 29, 2021

bedevere-bot removed the needs backport to 3.9 label Apr 29, 2021

bedevere-bot removed the needs backport to 3.8 label Apr 29, 2021

bedevere-bot removed the needs backport to 3.7 label Apr 29, 2021

bedevere-bot removed the needs backport to 3.6 label Apr 29, 2021

theta682 mentioned this pull request Feb 21, 2022

Fix CVE-2022-0391 IronLanguages/ironpython2#808

Merged

slozier pushed a commit to IronLanguages/ironpython2 that referenced this pull request Feb 26, 2022

Fix CVE-2022-0391 (#808)

aa0526d

Implement the fix from the upstream python/cpython#25595

brencass mentioned this pull request Apr 6, 2022

Python Security Vulnerability CVE-2022-0391 DynamoDS/Dynamo#12782

Open

PhilGrayson mentioned this pull request Jun 15, 2022

Fix for changes in urlsplit henrysher/cob#14

Open

orsenthil mentioned this pull request Aug 25, 2022

[security] CVE-2022-0391: urllib.parse should sanitize urls containing ASCII newline and tabs. #88048

Closed

bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. #25595

bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. #25595

orsenthil commented Apr 25, 2021 •

edited

gpshead left a comment

bedevere-bot commented Apr 28, 2021

miss-islington commented Apr 29, 2021

bedevere-bot commented Apr 29, 2021

bedevere-bot commented Apr 29, 2021

bedevere-bot commented Apr 29, 2021

bedevere-bot commented Apr 29, 2021

bedevere-bot commented May 5, 2021

bedevere-bot commented May 5, 2021

bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. #25595

bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. #25595

Conversation

orsenthil commented Apr 25, 2021 • edited

gpshead left a comment

bedevere-bot commented Apr 28, 2021

miss-islington commented Apr 29, 2021

bedevere-bot commented Apr 29, 2021

bedevere-bot commented Apr 29, 2021

bedevere-bot commented Apr 29, 2021

bedevere-bot commented Apr 29, 2021

bedevere-bot commented May 5, 2021

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

bedevere-bot commented May 5, 2021

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

orsenthil commented Apr 25, 2021 •

edited