Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf48a96a

Browse files
miss-islingtonillia-vgpshead
authored
[3.10] [3.11]pythongh-102153: Start stripping C0 control and space chars inurlsplit (pythonGH-102508) (pythonGH-104575) (python#104592)
pythongh-102153: Start stripping C0 control and space chars in `urlsplit` (pythonGH-102508)`urllib.parse.urlsplit` has already been respecting the WHATWG spec a bitpythonGH-25595.This adds more sanitizing to respect the "Remove any leading C0 control or space from input" [rule](https://url.spec.whatwg.org/GH-url-parsing:~:text=Remove%20any%20leading%20and%20trailing%20C0%20control%20or%20space%20from%20input.) in response to [CVE-2023-24329](https://nvd.nist.gov/vuln/detail/CVE-2023-24329).I simplified the docs by eliding the state of the world explanatoryparagraph in this security release only backport. (people will seethat in the mainline /3/ docs)---------(cherry picked from commit2f630e1)(cherry picked from commit610cc0a)Co-authored-by: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com>Co-authored-by: Illia Volochii <illia.volochii@gmail.com>Co-authored-by: Gregory P. Smith [Google] <greg@krypto.org>
1 parent425065b commitf48a96a

File tree

4 files changed

+111
-3
lines changed

4 files changed

+111
-3
lines changed

‎Doc/library/urllib.parse.rst

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ or on combining URL components into a URL string.
159159
ParseResult(scheme='http', netloc='www.cwi.nl:80', path='/%7Eguido/Python.html',
160160
params='', query='', fragment='')
161161

162+
..warning::
163+
164+
:func:`urlparse` does not perform validation. See:ref:`URL parsing
165+
security <url-parsing-security>` for details.
162166

163167
..versionchanged::3.2
164168
Added IPv6 URL parsing capabilities.
@@ -324,8 +328,14 @@ or on combining URL components into a URL string.
324328
``#``, ``@``, or ``:`` will raise a:exc:`ValueError`. If the URL is
325329
decomposed before parsing, no error will be raised.
326330

327-
Following the `WHATWG spec`_ that updates RFC 3986, ASCII newline
328-
``\n``, ``\r`` and tab ``\t`` characters are stripped from the URL.
331+
Following some of the `WHATWG spec`_ that updates RFC 3986, leading C0
332+
control and space characters are stripped from the URL. ``\n``,
333+
``\r`` and tab ``\t`` characters are removed from the URL at any position.
334+
335+
..warning::
336+
337+
:func:`urlsplit` does not perform validation. See:ref:`URL parsing
338+
security <url-parsing-security>` for details.
329339

330340
..versionchanged::3.6
331341
Out-of-range port numbers now raise:exc:`ValueError`, instead of
@@ -338,6 +348,9 @@ or on combining URL components into a URL string.
338348
..versionchanged::3.10
339349
ASCII newline and tab characters are stripped from the URL.
340350

351+
..versionchanged::3.10.12
352+
Leading WHATWG C0 control and space characters are stripped from the URL.
353+
341354
.. _WHATWG spec:https://url.spec.whatwg.org/#concept-basic-url-parser
342355

343356
..function::urlunsplit(parts)
@@ -414,6 +427,27 @@ or on combining URL components into a URL string.
414427
or ``scheme://host/path``). If *url* is not a wrapped URL, it is returned
415428
without changes.
416429

430+
.. _url-parsing-security:
431+
432+
URL parsing security
433+
--------------------
434+
435+
The:func:`urlsplit` and:func:`urlparse` APIs do not perform **validation** of
436+
inputs. They may not raise errors on inputs that other applications consider
437+
invalid. They may also succeed on some inputs that might not be considered
438+
URLs elsewhere. Their purpose is for practical functionality rather than
439+
purity.
440+
441+
Instead of raising an exception on unusual input, they may instead return some
442+
component parts as empty strings. Or components may contain more than perhaps
443+
they should.
444+
445+
We recommend that users of these APIs where the values may be used anywhere
446+
with security implications code defensively. Do some verification within your
447+
code before trusting a returned component part. Does that ``scheme`` make
448+
sense? Is that a sensible ``path``? Is there anything strange about that
449+
``hostname``? etc.
450+
417451
.. _parsing-ascii-encoded-bytes:
418452

419453
Parsing ASCII Encoded Bytes

‎Lib/test/test_urlparse.py

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,14 +649,73 @@ def test_urlsplit_remove_unsafe_bytes(self):
649649
self.assertEqual(p.scheme,"http")
650650
self.assertEqual(p.geturl(),"http://www.python.org/#"diff-bb89d64e7413a637b421609f9a4f532c294037171c3f0150479930acf30425fa-651-651-0" data-selected="false" role="gridcell" tabindex="-1" valign="top">651
651

652+
deftest_urlsplit_strip_url(self):
653+
noise=bytes(range(0,0x20+1))
654+
base_url="http://User:Pass@www.python.org:080/doc/?query=yes#frag"
655+
656+
url=noise.decode("utf-8")+base_url
657+
p=urllib.parse.urlsplit(url)
658+
self.assertEqual(p.scheme,"http")
659+
self.assertEqual(p.netloc,"User:Pass@www.python.org:080")
660+
self.assertEqual(p.path,"/doc/")
661+
self.assertEqual(p.query,"query=yes")
662+
self.assertEqual(p.fragment,"frag")
663+
self.assertEqual(p.username,"User")
664+
self.assertEqual(p.password,"Pass")
665+
self.assertEqual(p.hostname,"www.python.org")
666+
self.assertEqual(p.port,80)
667+
self.assertEqual(p.geturl(),base_url)
668+
669+
url=noise+base_url.encode("utf-8")
670+
p=urllib.parse.urlsplit(url)
671+
self.assertEqual(p.scheme,b"http")
672+
self.assertEqual(p.netloc,b"User:Pass@www.python.org:080")
673+
self.assertEqual(p.path,b"/doc/")
674+
self.assertEqual(p.query,b"query=yes")
675+
self.assertEqual(p.fragment,b"frag")
676+
self.assertEqual(p.username,b"User")
677+
self.assertEqual(p.password,b"Pass")
678+
self.assertEqual(p.hostname,b"www.python.org")
679+
self.assertEqual(p.port,80)
680+
self.assertEqual(p.geturl(),base_url.encode("utf-8"))
681+
682+
# Test that trailing space is preserved as some applications rely on
683+
# this within query strings.
684+
query_spaces_url="https://www.python.org:88/doc/?query= "
685+
p=urllib.parse.urlsplit(noise.decode("utf-8")+query_spaces_url)
686+
self.assertEqual(p.scheme,"https")
687+
self.assertEqual(p.netloc,"www.python.org:88")
688+
self.assertEqual(p.path,"/doc/")
689+
self.assertEqual(p.query,"query= ")
690+
self.assertEqual(p.port,88)
691+
self.assertEqual(p.geturl(),query_spaces_url)
692+
693+
p=urllib.parse.urlsplit("www.pypi.org ")
694+
# That "hostname" gets considered a "path" due to the
695+
# trailing space and our existing logic... YUCK...
696+
# and re-assembles via geturl aka unurlsplit into the original.
697+
# django.core.validators.URLValidator (at least through v3.2) relies on
698+
# this, for better or worse, to catch it in a ValidationError via its
699+
# regular expressions.
700+
# Here we test the basic round trip concept of such a trailing space.
701+
self.assertEqual(urllib.parse.urlunsplit(p),"www.pypi.org ")
702+
703+
# with scheme as cache-key
704+
url="//www.python.org/"
705+
scheme=noise.decode("utf-8")+"https"+noise.decode("utf-8")
706+
for_inrange(2):
707+
p=urllib.parse.urlsplit(url,scheme=scheme)
708+
self.assertEqual(p.scheme,"https")
709+
self.assertEqual(p.geturl(),"https://www.python.org/")
710+
652711
deftest_attributes_bad_port(self):
653712
"""Check handling of invalid ports."""
654713
forbytesin (False,True):
655714
forparsein (urllib.parse.urlsplit,urllib.parse.urlparse):
656715
forportin ("foo","1.5","-1","0x10","-0","1_1"," 1","1 ","६"):
657716
withself.subTest(bytes=bytes,parse=parse,port=port):
658717
netloc="www.example.net:"+port
659-
url="http://"+netloc
718+
url="http://"+netloc+"/"
660719
ifbytes:
661720
ifnetloc.isascii()andport.isascii():
662721
netloc=netloc.encode("ascii")

‎Lib/urllib/parse.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
scenarios for parsing, and for backward compatibility purposes, some
2626
parsing quirks from older RFCs are retained. The testcases in
2727
test_urlparse.py provides a good indicator of parsing behavior.
28+
29+
The WHATWG URL Parser spec should also be considered. We are not compliant with
30+
it either due to existing user code API behavior expectations (Hyrum's Law).
31+
It serves as a useful guide when making changes.
2832
"""
2933

3034
importre
@@ -78,6 +82,10 @@
7882
'0123456789'
7983
'+-.')
8084

85+
# Leading and trailing C0 control and space to be stripped per WHATWG spec.
86+
# == "".join([chr(i) for i in range(0, 0x20 + 1)])
87+
_WHATWG_C0_CONTROL_OR_SPACE='\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f '
88+
8189
# Unsafe bytes to be removed per WHATWG spec
8290
_UNSAFE_URL_BYTES_TO_REMOVE= ['\t','\r','\n']
8391

@@ -455,6 +463,10 @@ def urlsplit(url, scheme='', allow_fragments=True):
455463
"""
456464

457465
url,scheme,_coerce_result=_coerce_args(url,scheme)
466+
# Only lstrip url as some applications rely on preserving trailing space.
467+
# (https://url.spec.whatwg.org/#concept-basic-url-parser would strip both)
468+
url=url.lstrip(_WHATWG_C0_CONTROL_OR_SPACE)
469+
scheme=scheme.strip(_WHATWG_C0_CONTROL_OR_SPACE)
458470

459471
forbin_UNSAFE_URL_BYTES_TO_REMOVE:
460472
url=url.replace(b,"")
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:func:`urllib.parse.urlsplit` now strips leading C0 control and space
2+
characters following the specification for URLs defined by WHATWG in
3+
response to CVE-2023-24329. Patch by Illia Volochii.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp