You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
685 lines
32 KiB
685 lines
32 KiB
2 years ago
|
commit 9e77ec82c40ab59846f9447b7c483e7b8e368b16
|
||
|
Author: Petr Viktorin <pviktori@redhat.com>
|
||
|
Date: Thu Mar 4 13:59:56 2021 +0100
|
||
|
|
||
|
CVE-2021-23336: Add `separator` argument to parse_qs; warn with default
|
||
|
|
||
|
Partially backports https://bugs.python.org/issue42967 : [security] Address a web cache-poisoning issue reported in urllib.parse.parse_qsl().
|
||
|
However, this solution is different than the upstream solution in Python 3.6.13.
|
||
|
|
||
|
An optional argument seperator is added to specify the separator.
|
||
|
It is recommended to set it to '&' or ';' to match the application or proxy in use.
|
||
|
The default can be set with an env variable of a config file.
|
||
|
If neither the argument, env var or config file specifies a separator, "&" is used
|
||
|
but a warning is raised if parse_qs is used on input that contains ';'.
|
||
|
|
||
|
Co-authors of the upstream change (who do not necessarily agree with this):
|
||
|
Co-authored-by: Adam Goldschmidt <adamgold7@gmail.com>
|
||
|
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
|
||
|
Co-authored-by: Éric Araujo <merwok@netwok.org>
|
||
|
|
||
|
diff --git a/Doc/library/cgi.rst b/Doc/library/cgi.rst
|
||
|
index 41219eeaaba..ddecc0af23a 100644
|
||
|
--- a/Doc/library/cgi.rst
|
||
|
+++ b/Doc/library/cgi.rst
|
||
|
@@ -277,13 +277,12 @@ These are useful if you want more control, or if you want to employ some of the
|
||
|
algorithms implemented in this module in other circumstances.
|
||
|
|
||
|
|
||
|
-.. function:: parse(fp=None, environ=os.environ, keep_blank_values=False, strict_parsing=False)
|
||
|
+.. function:: parse(fp=None, environ=os.environ, keep_blank_values=False, strict_parsing=False, separator=None)
|
||
|
|
||
|
Parse a query in the environment or from a file (the file defaults to
|
||
|
- ``sys.stdin``). The *keep_blank_values* and *strict_parsing* parameters are
|
||
|
+ ``sys.stdin``). The *keep_blank_values*, *strict_parsing* and *separator* parameters are
|
||
|
passed to :func:`urllib.parse.parse_qs` unchanged.
|
||
|
|
||
|
-
|
||
|
.. function:: parse_qs(qs, keep_blank_values=False, strict_parsing=False)
|
||
|
|
||
|
This function is deprecated in this module. Use :func:`urllib.parse.parse_qs`
|
||
|
@@ -308,7 +307,6 @@ algorithms implemented in this module in other circumstances.
|
||
|
Note that this does not parse nested multipart parts --- use
|
||
|
:class:`FieldStorage` for that.
|
||
|
|
||
|
-
|
||
|
.. function:: parse_header(string)
|
||
|
|
||
|
Parse a MIME header (such as :mailheader:`Content-Type`) into a main value and a
|
||
|
diff --git a/Doc/library/urllib.parse.rst b/Doc/library/urllib.parse.rst
|
||
|
index 647af613a31..bcab7c142bc 100644
|
||
|
--- a/Doc/library/urllib.parse.rst
|
||
|
+++ b/Doc/library/urllib.parse.rst
|
||
|
@@ -143,7 +143,7 @@ or on combining URL components into a URL string.
|
||
|
now raise :exc:`ValueError`.
|
||
|
|
||
|
|
||
|
-.. function:: parse_qs(qs, keep_blank_values=False, strict_parsing=False, encoding='utf-8', errors='replace', max_num_fields=None)
|
||
|
+.. function:: parse_qs(qs, keep_blank_values=False, strict_parsing=False, encoding='utf-8', errors='replace', max_num_fields=None, separator=None)
|
||
|
|
||
|
Parse a query string given as a string argument (data of type
|
||
|
:mimetype:`application/x-www-form-urlencoded`). Data are returned as a
|
||
|
@@ -168,6 +168,15 @@ or on combining URL components into a URL string.
|
||
|
read. If set, then throws a :exc:`ValueError` if there are more than
|
||
|
*max_num_fields* fields read.
|
||
|
|
||
|
+ The optional argument *separator* is the symbol to use for separating the
|
||
|
+ query arguments. It is recommended to set it to ``'&'`` or ``';'``.
|
||
|
+ It defaults to ``'&'``; a warning is raised if this default is used.
|
||
|
+ This default may be changed with the following environment variable settings:
|
||
|
+
|
||
|
+ - ``PYTHON_URLLIB_QS_SEPARATOR='&'``: use only ``&`` as separator, without warning (as in Python 3.6.13+ or 3.10)
|
||
|
+ - ``PYTHON_URLLIB_QS_SEPARATOR=';'``: use only ``;`` as separator
|
||
|
+ - ``PYTHON_URLLIB_QS_SEPARATOR=legacy``: use both ``&`` and ``;`` (as in previous versions of Python)
|
||
|
+
|
||
|
Use the :func:`urllib.parse.urlencode` function (with the ``doseq``
|
||
|
parameter set to ``True``) to convert such dictionaries into query
|
||
|
strings.
|
||
|
@@ -204,6 +213,9 @@ or on combining URL components into a URL string.
|
||
|
read. If set, then throws a :exc:`ValueError` if there are more than
|
||
|
*max_num_fields* fields read.
|
||
|
|
||
|
+ The optional argument *separator* is the symbol to use for separating the
|
||
|
+ query arguments. It works as in :py:func:`parse_qs`.
|
||
|
+
|
||
|
Use the :func:`urllib.parse.urlencode` function to convert such lists of pairs into
|
||
|
query strings.
|
||
|
|
||
|
@@ -213,7 +225,6 @@ or on combining URL components into a URL string.
|
||
|
.. versionchanged:: 3.6.8
|
||
|
Added *max_num_fields* parameter.
|
||
|
|
||
|
-
|
||
|
.. function:: urlunparse(parts)
|
||
|
|
||
|
Construct a URL from a tuple as returned by ``urlparse()``. The *parts*
|
||
|
diff --git a/Lib/cgi.py b/Lib/cgi.py
|
||
|
index 56f243e09f0..5ab2a5d6af6 100755
|
||
|
--- a/Lib/cgi.py
|
||
|
+++ b/Lib/cgi.py
|
||
|
@@ -117,7 +117,8 @@ log = initlog # The current logging function
|
||
|
# 0 ==> unlimited input
|
||
|
maxlen = 0
|
||
|
|
||
|
-def parse(fp=None, environ=os.environ, keep_blank_values=0, strict_parsing=0):
|
||
|
+def parse(fp=None, environ=os.environ, keep_blank_values=0,
|
||
|
+ strict_parsing=0, separator=None):
|
||
|
"""Parse a query in the environment or from a file (default stdin)
|
||
|
|
||
|
Arguments, all optional:
|
||
|
@@ -136,6 +137,8 @@ def parse(fp=None, environ=os.environ, keep_blank_values=0, strict_parsing=0):
|
||
|
strict_parsing: flag indicating what to do with parsing errors.
|
||
|
If false (the default), errors are silently ignored.
|
||
|
If true, errors raise a ValueError exception.
|
||
|
+
|
||
|
+ separator: str. The symbol to use for separating the query arguments.
|
||
|
"""
|
||
|
if fp is None:
|
||
|
fp = sys.stdin
|
||
|
@@ -156,7 +159,7 @@ def parse(fp=None, environ=os.environ, keep_blank_values=0, strict_parsing=0):
|
||
|
if environ['REQUEST_METHOD'] == 'POST':
|
||
|
ctype, pdict = parse_header(environ['CONTENT_TYPE'])
|
||
|
if ctype == 'multipart/form-data':
|
||
|
- return parse_multipart(fp, pdict)
|
||
|
+ return parse_multipart(fp, pdict, separator=separator)
|
||
|
elif ctype == 'application/x-www-form-urlencoded':
|
||
|
clength = int(environ['CONTENT_LENGTH'])
|
||
|
if maxlen and clength > maxlen:
|
||
|
@@ -182,21 +185,21 @@ def parse(fp=None, environ=os.environ, keep_blank_values=0, strict_parsing=0):
|
||
|
return urllib.parse.parse_qs(qs, keep_blank_values, strict_parsing,
|
||
|
encoding=encoding)
|
||
|
|
||
|
-
|
||
|
# parse query string function called from urlparse,
|
||
|
# this is done in order to maintain backward compatibility.
|
||
|
-
|
||
|
-def parse_qs(qs, keep_blank_values=0, strict_parsing=0):
|
||
|
+def parse_qs(qs, keep_blank_values=0, strict_parsing=0, separator=None):
|
||
|
"""Parse a query given as a string argument."""
|
||
|
warn("cgi.parse_qs is deprecated, use urllib.parse.parse_qs instead",
|
||
|
DeprecationWarning, 2)
|
||
|
- return urllib.parse.parse_qs(qs, keep_blank_values, strict_parsing)
|
||
|
+ return urllib.parse.parse_qs(qs, keep_blank_values, strict_parsing,
|
||
|
+ separator=separator)
|
||
|
|
||
|
-def parse_qsl(qs, keep_blank_values=0, strict_parsing=0):
|
||
|
+def parse_qsl(qs, keep_blank_values=0, strict_parsing=0, separator=None):
|
||
|
"""Parse a query given as a string argument."""
|
||
|
warn("cgi.parse_qsl is deprecated, use urllib.parse.parse_qsl instead",
|
||
|
DeprecationWarning, 2)
|
||
|
- return urllib.parse.parse_qsl(qs, keep_blank_values, strict_parsing)
|
||
|
+ return urllib.parse.parse_qsl(qs, keep_blank_values, strict_parsing,
|
||
|
+ separator=separator)
|
||
|
|
||
|
def parse_multipart(fp, pdict):
|
||
|
"""Parse multipart input.
|
||
|
@@ -297,7 +300,6 @@ def parse_multipart(fp, pdict):
|
||
|
|
||
|
return partdict
|
||
|
|
||
|
-
|
||
|
def _parseparam(s):
|
||
|
while s[:1] == ';':
|
||
|
s = s[1:]
|
||
|
@@ -405,7 +407,7 @@ class FieldStorage:
|
||
|
def __init__(self, fp=None, headers=None, outerboundary=b'',
|
||
|
environ=os.environ, keep_blank_values=0, strict_parsing=0,
|
||
|
limit=None, encoding='utf-8', errors='replace',
|
||
|
- max_num_fields=None):
|
||
|
+ max_num_fields=None, separator=None):
|
||
|
"""Constructor. Read multipart/* until last part.
|
||
|
|
||
|
Arguments, all optional:
|
||
|
@@ -453,6 +455,7 @@ class FieldStorage:
|
||
|
self.keep_blank_values = keep_blank_values
|
||
|
self.strict_parsing = strict_parsing
|
||
|
self.max_num_fields = max_num_fields
|
||
|
+ self.separator = separator
|
||
|
if 'REQUEST_METHOD' in environ:
|
||
|
method = environ['REQUEST_METHOD'].upper()
|
||
|
self.qs_on_post = None
|
||
|
@@ -678,7 +681,7 @@ class FieldStorage:
|
||
|
query = urllib.parse.parse_qsl(
|
||
|
qs, self.keep_blank_values, self.strict_parsing,
|
||
|
encoding=self.encoding, errors=self.errors,
|
||
|
- max_num_fields=self.max_num_fields)
|
||
|
+ max_num_fields=self.max_num_fields, separator=self.separator)
|
||
|
self.list = [MiniFieldStorage(key, value) for key, value in query]
|
||
|
self.skip_lines()
|
||
|
|
||
|
@@ -694,7 +697,7 @@ class FieldStorage:
|
||
|
query = urllib.parse.parse_qsl(
|
||
|
self.qs_on_post, self.keep_blank_values, self.strict_parsing,
|
||
|
encoding=self.encoding, errors=self.errors,
|
||
|
- max_num_fields=self.max_num_fields)
|
||
|
+ max_num_fields=self.max_num_fields, separator=self.separator)
|
||
|
self.list.extend(MiniFieldStorage(key, value) for key, value in query)
|
||
|
|
||
|
klass = self.FieldStorageClass or self.__class__
|
||
|
@@ -736,7 +739,8 @@ class FieldStorage:
|
||
|
|
||
|
part = klass(self.fp, headers, ib, environ, keep_blank_values,
|
||
|
strict_parsing,self.limit-self.bytes_read,
|
||
|
- self.encoding, self.errors, max_num_fields)
|
||
|
+ self.encoding, self.errors, max_num_fields,
|
||
|
+ separator=self.separator)
|
||
|
|
||
|
if max_num_fields is not None:
|
||
|
max_num_fields -= 1
|
||
|
diff --git a/Lib/test/test_cgi.py b/Lib/test/test_cgi.py
|
||
|
index b3e2d4cce8e..5ae3e085e1e 100644
|
||
|
--- a/Lib/test/test_cgi.py
|
||
|
+++ b/Lib/test/test_cgi.py
|
||
|
@@ -55,12 +55,9 @@ parse_strict_test_cases = [
|
||
|
("", ValueError("bad query field: ''")),
|
||
|
("&", ValueError("bad query field: ''")),
|
||
|
("&&", ValueError("bad query field: ''")),
|
||
|
- (";", ValueError("bad query field: ''")),
|
||
|
- (";&;", ValueError("bad query field: ''")),
|
||
|
# Should the next few really be valid?
|
||
|
("=", {}),
|
||
|
("=&=", {}),
|
||
|
- ("=;=", {}),
|
||
|
# This rest seem to make sense
|
||
|
("=a", {'': ['a']}),
|
||
|
("&=a", ValueError("bad query field: ''")),
|
||
|
@@ -75,8 +72,6 @@ parse_strict_test_cases = [
|
||
|
("a=a+b&b=b+c", {'a': ['a b'], 'b': ['b c']}),
|
||
|
("a=a+b&a=b+a", {'a': ['a b', 'b a']}),
|
||
|
("x=1&y=2.0&z=2-3.%2b0", {'x': ['1'], 'y': ['2.0'], 'z': ['2-3.+0']}),
|
||
|
- ("x=1;y=2.0&z=2-3.%2b0", {'x': ['1'], 'y': ['2.0'], 'z': ['2-3.+0']}),
|
||
|
- ("x=1;y=2.0;z=2-3.%2b0", {'x': ['1'], 'y': ['2.0'], 'z': ['2-3.+0']}),
|
||
|
("Hbc5161168c542333633315dee1182227:key_store_seqid=400006&cuyer=r&view=bustomer&order_id=0bb2e248638833d48cb7fed300000f1b&expire=964546263&lobale=en-US&kid=130003.300038&ss=env",
|
||
|
{'Hbc5161168c542333633315dee1182227:key_store_seqid': ['400006'],
|
||
|
'cuyer': ['r'],
|
||
|
@@ -164,6 +159,35 @@ class CgiTests(unittest.TestCase):
|
||
|
|
||
|
env = {'QUERY_STRING': orig}
|
||
|
fs = cgi.FieldStorage(environ=env)
|
||
|
+ if isinstance(expect, dict):
|
||
|
+ # test dict interface
|
||
|
+ self.assertEqual(len(expect), len(fs))
|
||
|
+ self.assertCountEqual(expect.keys(), fs.keys())
|
||
|
+ self.assertEqual(fs.getvalue("nonexistent field", "default"), "default")
|
||
|
+ # test individual fields
|
||
|
+ for key in expect.keys():
|
||
|
+ expect_val = expect[key]
|
||
|
+ self.assertIn(key, fs)
|
||
|
+ if len(expect_val) > 1:
|
||
|
+ self.assertEqual(fs.getvalue(key), expect_val)
|
||
|
+ else:
|
||
|
+ self.assertEqual(fs.getvalue(key), expect_val[0])
|
||
|
+
|
||
|
+ def test_separator(self):
|
||
|
+ parse_semicolon = [
|
||
|
+ ("x=1;y=2.0", {'x': ['1'], 'y': ['2.0']}),
|
||
|
+ ("x=1;y=2.0;z=2-3.%2b0", {'x': ['1'], 'y': ['2.0'], 'z': ['2-3.+0']}),
|
||
|
+ (";", ValueError("bad query field: ''")),
|
||
|
+ (";;", ValueError("bad query field: ''")),
|
||
|
+ ("=;a", ValueError("bad query field: 'a'")),
|
||
|
+ (";b=a", ValueError("bad query field: ''")),
|
||
|
+ ("b;=a", ValueError("bad query field: 'b'")),
|
||
|
+ ("a=a+b;b=b+c", {'a': ['a b'], 'b': ['b c']}),
|
||
|
+ ("a=a+b;a=b+a", {'a': ['a b', 'b a']}),
|
||
|
+ ]
|
||
|
+ for orig, expect in parse_semicolon:
|
||
|
+ env = {'QUERY_STRING': orig}
|
||
|
+ fs = cgi.FieldStorage(separator=';', environ=env)
|
||
|
if isinstance(expect, dict):
|
||
|
# test dict interface
|
||
|
self.assertEqual(len(expect), len(fs))
|
||
|
diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py
|
||
|
index 68f633ca3a7..1ec86ba0fc2 100644
|
||
|
--- a/Lib/test/test_urlparse.py
|
||
|
+++ b/Lib/test/test_urlparse.py
|
||
|
@@ -2,6 +2,11 @@ import sys
|
||
|
import unicodedata
|
||
|
import unittest
|
||
|
import urllib.parse
|
||
|
+from test.support import EnvironmentVarGuard
|
||
|
+from warnings import catch_warnings
|
||
|
+import tempfile
|
||
|
+import contextlib
|
||
|
+import os.path
|
||
|
|
||
|
RFC1808_BASE = "http://a/b/c/d;p?q#f"
|
||
|
RFC2396_BASE = "http://a/b/c/d;p?q"
|
||
|
@@ -32,6 +37,9 @@ parse_qsl_test_cases = [
|
||
|
(b"&a=b", [(b'a', b'b')]),
|
||
|
(b"a=a+b&b=b+c", [(b'a', b'a b'), (b'b', b'b c')]),
|
||
|
(b"a=1&a=2", [(b'a', b'1'), (b'a', b'2')]),
|
||
|
+]
|
||
|
+
|
||
|
+parse_qsl_test_cases_semicolon = [
|
||
|
(";", []),
|
||
|
(";;", []),
|
||
|
(";a=b", [('a', 'b')]),
|
||
|
@@ -44,6 +52,21 @@ parse_qsl_test_cases = [
|
||
|
(b"a=1;a=2", [(b'a', b'1'), (b'a', b'2')]),
|
||
|
]
|
||
|
|
||
|
+parse_qsl_test_cases_legacy = [
|
||
|
+ (b"a=1;a=2&a=3", [(b'a', b'1'), (b'a', b'2'), (b'a', b'3')]),
|
||
|
+ (b"a=1;b=2&c=3", [(b'a', b'1'), (b'b', b'2'), (b'c', b'3')]),
|
||
|
+ (b"a=1&b=2&c=3;", [(b'a', b'1'), (b'b', b'2'), (b'c', b'3')]),
|
||
|
+]
|
||
|
+
|
||
|
+parse_qsl_test_cases_warn = [
|
||
|
+ (";a=b", [(';a', 'b')]),
|
||
|
+ ("a=a+b;b=b+c", [('a', 'a b;b=b c')]),
|
||
|
+ (b";a=b", [(b';a', b'b')]),
|
||
|
+ (b"a=a+b;b=b+c", [(b'a', b'a b;b=b c')]),
|
||
|
+ ("a=1;a=2&a=3", [('a', '1;a=2'), ('a', '3')]),
|
||
|
+ (b"a=1;a=2&a=3", [(b'a', b'1;a=2'), (b'a', b'3')]),
|
||
|
+]
|
||
|
+
|
||
|
# Each parse_qs testcase is a two-tuple that contains
|
||
|
# a string with the query and a dictionary with the expected result.
|
||
|
|
||
|
@@ -68,6 +91,9 @@ parse_qs_test_cases = [
|
||
|
(b"&a=b", {b'a': [b'b']}),
|
||
|
(b"a=a+b&b=b+c", {b'a': [b'a b'], b'b': [b'b c']}),
|
||
|
(b"a=1&a=2", {b'a': [b'1', b'2']}),
|
||
|
+]
|
||
|
+
|
||
|
+parse_qs_test_cases_semicolon = [
|
||
|
(";", {}),
|
||
|
(";;", {}),
|
||
|
(";a=b", {'a': ['b']}),
|
||
|
@@ -80,6 +106,24 @@ parse_qs_test_cases = [
|
||
|
(b"a=1;a=2", {b'a': [b'1', b'2']}),
|
||
|
]
|
||
|
|
||
|
+parse_qs_test_cases_legacy = [
|
||
|
+ ("a=1;a=2&a=3", {'a': ['1', '2', '3']}),
|
||
|
+ ("a=1;b=2&c=3", {'a': ['1'], 'b': ['2'], 'c': ['3']}),
|
||
|
+ ("a=1&b=2&c=3;", {'a': ['1'], 'b': ['2'], 'c': ['3']}),
|
||
|
+ (b"a=1;a=2&a=3", {b'a': [b'1', b'2', b'3']}),
|
||
|
+ (b"a=1;b=2&c=3", {b'a': [b'1'], b'b': [b'2'], b'c': [b'3']}),
|
||
|
+ (b"a=1&b=2&c=3;", {b'a': [b'1'], b'b': [b'2'], b'c': [b'3']}),
|
||
|
+]
|
||
|
+
|
||
|
+parse_qs_test_cases_warn = [
|
||
|
+ (";a=b", {';a': ['b']}),
|
||
|
+ ("a=a+b;b=b+c", {'a': ['a b;b=b c']}),
|
||
|
+ (b";a=b", {b';a': [b'b']}),
|
||
|
+ (b"a=a+b;b=b+c", {b'a':[ b'a b;b=b c']}),
|
||
|
+ ("a=1;a=2&a=3", {'a': ['1;a=2', '3']}),
|
||
|
+ (b"a=1;a=2&a=3", {b'a': [b'1;a=2', b'3']}),
|
||
|
+]
|
||
|
+
|
||
|
class UrlParseTestCase(unittest.TestCase):
|
||
|
|
||
|
def checkRoundtrips(self, url, parsed, split):
|
||
|
@@ -152,6 +196,40 @@ class UrlParseTestCase(unittest.TestCase):
|
||
|
self.assertEqual(result, expect_without_blanks,
|
||
|
"Error parsing %r" % orig)
|
||
|
|
||
|
+ def test_qs_default_warn(self):
|
||
|
+ for orig, expect in parse_qs_test_cases_warn:
|
||
|
+ with self.subTest(orig=orig, expect=expect):
|
||
|
+ with catch_warnings(record=True) as w:
|
||
|
+ result = urllib.parse.parse_qs(orig, keep_blank_values=True)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 1)
|
||
|
+ self.assertEqual(w[0].category, urllib.parse._QueryStringSeparatorWarning)
|
||
|
+
|
||
|
+ def test_qsl_default_warn(self):
|
||
|
+ for orig, expect in parse_qsl_test_cases_warn:
|
||
|
+ with self.subTest(orig=orig, expect=expect):
|
||
|
+ with catch_warnings(record=True) as w:
|
||
|
+ result = urllib.parse.parse_qsl(orig, keep_blank_values=True)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 1)
|
||
|
+ self.assertEqual(w[0].category, urllib.parse._QueryStringSeparatorWarning)
|
||
|
+
|
||
|
+ def test_default_qs_no_warnings(self):
|
||
|
+ for orig, expect in parse_qs_test_cases:
|
||
|
+ with self.subTest(orig=orig, expect=expect):
|
||
|
+ with catch_warnings(record=True) as w:
|
||
|
+ result = urllib.parse.parse_qs(orig, keep_blank_values=True)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+
|
||
|
+ def test_default_qsl_no_warnings(self):
|
||
|
+ for orig, expect in parse_qsl_test_cases:
|
||
|
+ with self.subTest(orig=orig, expect=expect):
|
||
|
+ with catch_warnings(record=True) as w:
|
||
|
+ result = urllib.parse.parse_qsl(orig, keep_blank_values=True)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+
|
||
|
def test_roundtrips(self):
|
||
|
str_cases = [
|
||
|
('file:///tmp/junk.txt',
|
||
|
@@ -885,8 +963,151 @@ class UrlParseTestCase(unittest.TestCase):
|
||
|
with self.assertRaises(ValueError):
|
||
|
urllib.parse.parse_qs('&'.join(['a=a']*11), max_num_fields=10)
|
||
|
with self.assertRaises(ValueError):
|
||
|
- urllib.parse.parse_qs(';'.join(['a=a']*11), max_num_fields=10)
|
||
|
+ urllib.parse.parse_qs(';'.join(['a=a']*11), separator=';', max_num_fields=10)
|
||
|
+ with self.assertRaises(ValueError):
|
||
|
+ urllib.parse.parse_qs('SEP'.join(['a=a']*11), separator='SEP', max_num_fields=10)
|
||
|
urllib.parse.parse_qs('&'.join(['a=a']*10), max_num_fields=10)
|
||
|
+ urllib.parse.parse_qs(';'.join(['a=a']*10), separator=';', max_num_fields=10)
|
||
|
+ urllib.parse.parse_qs('SEP'.join(['a=a']*10), separator='SEP', max_num_fields=10)
|
||
|
+
|
||
|
+ def test_parse_qs_separator_bytes(self):
|
||
|
+ expected = {b'a': [b'1'], b'b': [b'2']}
|
||
|
+
|
||
|
+ result = urllib.parse.parse_qs(b'a=1;b=2', separator=b';')
|
||
|
+ self.assertEqual(result, expected)
|
||
|
+ result = urllib.parse.parse_qs(b'a=1;b=2', separator=';')
|
||
|
+ self.assertEqual(result, expected)
|
||
|
+ result = urllib.parse.parse_qs('a=1;b=2', separator=';')
|
||
|
+ self.assertEqual(result, {'a': ['1'], 'b': ['2']})
|
||
|
+
|
||
|
+ @contextlib.contextmanager
|
||
|
+ def _qsl_sep_config(self, sep):
|
||
|
+ """Context for the given parse_qsl default separator configured in config file"""
|
||
|
+ old_filename = urllib.parse._QS_SEPARATOR_CONFIG_FILENAME
|
||
|
+ urllib.parse._default_qs_separator = None
|
||
|
+ try:
|
||
|
+ with tempfile.TemporaryDirectory() as tmpdirname:
|
||
|
+ filename = os.path.join(tmpdirname, 'conf.cfg')
|
||
|
+ with open(filename, 'w') as file:
|
||
|
+ file.write(f'[parse_qs]\n')
|
||
|
+ file.write(f'PYTHON_URLLIB_QS_SEPARATOR = {sep}')
|
||
|
+ urllib.parse._QS_SEPARATOR_CONFIG_FILENAME = filename
|
||
|
+ yield
|
||
|
+ finally:
|
||
|
+ urllib.parse._QS_SEPARATOR_CONFIG_FILENAME = old_filename
|
||
|
+ urllib.parse._default_qs_separator = None
|
||
|
+
|
||
|
+ def test_parse_qs_separator_semicolon(self):
|
||
|
+ for orig, expect in parse_qs_test_cases_semicolon:
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='arg'):
|
||
|
+ result = urllib.parse.parse_qs(orig, separator=';')
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='env'):
|
||
|
+ with EnvironmentVarGuard() as environ, catch_warnings(record=True) as w:
|
||
|
+ environ['PYTHON_URLLIB_QS_SEPARATOR'] = ';'
|
||
|
+ result = urllib.parse.parse_qs(orig)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='conf'):
|
||
|
+ with self._qsl_sep_config(';'), catch_warnings(record=True) as w:
|
||
|
+ result = urllib.parse.parse_qs(orig)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+
|
||
|
+ def test_parse_qsl_separator_semicolon(self):
|
||
|
+ for orig, expect in parse_qsl_test_cases_semicolon:
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='arg'):
|
||
|
+ result = urllib.parse.parse_qsl(orig, separator=';')
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='env'):
|
||
|
+ with EnvironmentVarGuard() as environ, catch_warnings(record=True) as w:
|
||
|
+ environ['PYTHON_URLLIB_QS_SEPARATOR'] = ';'
|
||
|
+ result = urllib.parse.parse_qsl(orig)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='conf'):
|
||
|
+ with self._qsl_sep_config(';'), catch_warnings(record=True) as w:
|
||
|
+ result = urllib.parse.parse_qsl(orig)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+
|
||
|
+ def test_parse_qs_separator_legacy(self):
|
||
|
+ for orig, expect in parse_qs_test_cases_legacy:
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='env'):
|
||
|
+ with EnvironmentVarGuard() as environ, catch_warnings(record=True) as w:
|
||
|
+ environ['PYTHON_URLLIB_QS_SEPARATOR'] = 'legacy'
|
||
|
+ result = urllib.parse.parse_qs(orig)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='conf'):
|
||
|
+ with self._qsl_sep_config('legacy'), catch_warnings(record=True) as w:
|
||
|
+ result = urllib.parse.parse_qs(orig)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+
|
||
|
+ def test_parse_qsl_separator_legacy(self):
|
||
|
+ for orig, expect in parse_qsl_test_cases_legacy:
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='env'):
|
||
|
+ with EnvironmentVarGuard() as environ, catch_warnings(record=True) as w:
|
||
|
+ environ['PYTHON_URLLIB_QS_SEPARATOR'] = 'legacy'
|
||
|
+ result = urllib.parse.parse_qsl(orig)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+ with self.subTest(orig=orig, expect=expect, method='conf'):
|
||
|
+ with self._qsl_sep_config('legacy'), catch_warnings(record=True) as w:
|
||
|
+ result = urllib.parse.parse_qsl(orig)
|
||
|
+ self.assertEqual(result, expect, "Error parsing %r" % orig)
|
||
|
+ self.assertEqual(len(w), 0)
|
||
|
+
|
||
|
+ def test_parse_qs_separator_bad_value_env_or_config(self):
|
||
|
+ for bad_sep in '', 'abc', 'safe', '&;', 'SEP':
|
||
|
+ with self.subTest(bad_sep, method='env'):
|
||
|
+ with EnvironmentVarGuard() as environ, catch_warnings(record=True) as w:
|
||
|
+ environ['PYTHON_URLLIB_QS_SEPARATOR'] = bad_sep
|
||
|
+ with self.assertRaises(ValueError):
|
||
|
+ urllib.parse.parse_qsl('a=1;b=2')
|
||
|
+ with self.subTest(bad_sep, method='conf'):
|
||
|
+ with self._qsl_sep_config('bad_sep'), catch_warnings(record=True) as w:
|
||
|
+ with self.assertRaises(ValueError):
|
||
|
+ urllib.parse.parse_qsl('a=1;b=2')
|
||
|
+
|
||
|
+ def test_parse_qs_separator_bad_value_arg(self):
|
||
|
+ for bad_sep in True, {}, '':
|
||
|
+ with self.subTest(bad_sep):
|
||
|
+ with self.assertRaises(ValueError):
|
||
|
+ urllib.parse.parse_qsl('a=1;b=2', separator=bad_sep)
|
||
|
+
|
||
|
+ def test_parse_qs_separator_num_fields(self):
|
||
|
+ for qs, sep in (
|
||
|
+ ('a&b&c', '&'),
|
||
|
+ ('a;b;c', ';'),
|
||
|
+ ('a&b;c', 'legacy'),
|
||
|
+ ):
|
||
|
+ with self.subTest(qs=qs, sep=sep):
|
||
|
+ with EnvironmentVarGuard() as environ, catch_warnings(record=True) as w:
|
||
|
+ if sep != 'legacy':
|
||
|
+ with self.assertRaises(ValueError):
|
||
|
+ urllib.parse.parse_qsl(qs, separator=sep, max_num_fields=2)
|
||
|
+ if sep:
|
||
|
+ environ['PYTHON_URLLIB_QS_SEPARATOR'] = sep
|
||
|
+ with self.assertRaises(ValueError):
|
||
|
+ urllib.parse.parse_qsl(qs, max_num_fields=2)
|
||
|
+
|
||
|
+ def test_parse_qs_separator_priority(self):
|
||
|
+ # env variable trumps config file
|
||
|
+ with self._qsl_sep_config('~'), EnvironmentVarGuard() as environ:
|
||
|
+ environ['PYTHON_URLLIB_QS_SEPARATOR'] = '!'
|
||
|
+ result = urllib.parse.parse_qs('a=1!b=2~c=3')
|
||
|
+ self.assertEqual(result, {'a': ['1'], 'b': ['2~c=3']})
|
||
|
+ # argument trumps config file
|
||
|
+ with self._qsl_sep_config('~'):
|
||
|
+ result = urllib.parse.parse_qs('a=1$b=2~c=3', separator='$')
|
||
|
+ self.assertEqual(result, {'a': ['1'], 'b': ['2~c=3']})
|
||
|
+ # argument trumps env variable
|
||
|
+ with EnvironmentVarGuard() as environ:
|
||
|
+ environ['PYTHON_URLLIB_QS_SEPARATOR'] = '~'
|
||
|
+ result = urllib.parse.parse_qs('a=1$b=2~c=3', separator='$')
|
||
|
+ self.assertEqual(result, {'a': ['1'], 'b': ['2~c=3']})
|
||
|
|
||
|
def test_urlencode_sequences(self):
|
||
|
# Other tests incidentally urlencode things; test non-covered cases:
|
||
|
diff --git a/Lib/urllib/parse.py b/Lib/urllib/parse.py
|
||
|
index fa8827a9fa7..57b8fcf8bbd 100644
|
||
|
--- a/Lib/urllib/parse.py
|
||
|
+++ b/Lib/urllib/parse.py
|
||
|
@@ -28,6 +28,7 @@ test_urlparse.py provides a good indicator of parsing behavior.
|
||
|
"""
|
||
|
|
||
|
import re
|
||
|
+import os
|
||
|
import sys
|
||
|
import collections
|
||
|
|
||
|
@@ -644,7 +645,8 @@ def unquote(string, encoding='utf-8', errors='replace'):
|
||
|
|
||
|
|
||
|
def parse_qs(qs, keep_blank_values=False, strict_parsing=False,
|
||
|
- encoding='utf-8', errors='replace', max_num_fields=None):
|
||
|
+ encoding='utf-8', errors='replace', max_num_fields=None,
|
||
|
+ separator=None):
|
||
|
"""Parse a query given as a string argument.
|
||
|
|
||
|
Arguments:
|
||
|
@@ -673,7 +675,8 @@ def parse_qs(qs, keep_blank_values=False, strict_parsing=False,
|
||
|
parsed_result = {}
|
||
|
pairs = parse_qsl(qs, keep_blank_values, strict_parsing,
|
||
|
encoding=encoding, errors=errors,
|
||
|
- max_num_fields=max_num_fields)
|
||
|
+ max_num_fields=max_num_fields,
|
||
|
+ separator=separator)
|
||
|
for name, value in pairs:
|
||
|
if name in parsed_result:
|
||
|
parsed_result[name].append(value)
|
||
|
@@ -681,9 +684,16 @@ def parse_qs(qs, keep_blank_values=False, strict_parsing=False,
|
||
|
parsed_result[name] = [value]
|
||
|
return parsed_result
|
||
|
|
||
|
+class _QueryStringSeparatorWarning(RuntimeWarning):
|
||
|
+ """Warning for using default `separator` in parse_qs or parse_qsl"""
|
||
|
+
|
||
|
+# The default "separator" for parse_qsl can be specified in a config file.
|
||
|
+# It's cached after first read.
|
||
|
+_QS_SEPARATOR_CONFIG_FILENAME = '/etc/python/urllib.cfg'
|
||
|
+_default_qs_separator = None
|
||
|
|
||
|
def parse_qsl(qs, keep_blank_values=False, strict_parsing=False,
|
||
|
- encoding='utf-8', errors='replace', max_num_fields=None):
|
||
|
+ encoding='utf-8', errors='replace', max_num_fields=None, separator=None):
|
||
|
"""Parse a query given as a string argument.
|
||
|
|
||
|
Arguments:
|
||
|
@@ -710,15 +720,77 @@ def parse_qsl(qs, keep_blank_values=False, strict_parsing=False,
|
||
|
"""
|
||
|
qs, _coerce_result = _coerce_args(qs)
|
||
|
|
||
|
+ if isinstance(separator, bytes):
|
||
|
+ separator = separator.decode('ascii')
|
||
|
+
|
||
|
+ if (not separator or (not isinstance(separator, (str, bytes)))) and separator is not None:
|
||
|
+ raise ValueError("Separator must be of type string or bytes.")
|
||
|
+
|
||
|
+ # Used when both "&" and ";" act as separators. (Need a non-string value.)
|
||
|
+ _legacy = object()
|
||
|
+
|
||
|
+ if separator is None:
|
||
|
+ global _default_qs_separator
|
||
|
+ separator = _default_qs_separator
|
||
|
+ envvar_name = 'PYTHON_URLLIB_QS_SEPARATOR'
|
||
|
+ if separator is None:
|
||
|
+ # Set default separator from environment variable
|
||
|
+ separator = os.environ.get(envvar_name)
|
||
|
+ config_source = 'environment variable'
|
||
|
+ if separator is None:
|
||
|
+ # Set default separator from the configuration file
|
||
|
+ try:
|
||
|
+ file = open(_QS_SEPARATOR_CONFIG_FILENAME)
|
||
|
+ except FileNotFoundError:
|
||
|
+ pass
|
||
|
+ else:
|
||
|
+ with file:
|
||
|
+ import configparser
|
||
|
+ config = configparser.ConfigParser(
|
||
|
+ interpolation=None,
|
||
|
+ comment_prefixes=('#', ),
|
||
|
+ )
|
||
|
+ config.read_file(file)
|
||
|
+ separator = config.get('parse_qs', envvar_name, fallback=None)
|
||
|
+ _default_qs_separator = separator
|
||
|
+ config_source = _QS_SEPARATOR_CONFIG_FILENAME
|
||
|
+ if separator is None:
|
||
|
+ # The default is '&', but warn if not specified explicitly
|
||
|
+ if ';' in qs:
|
||
|
+ from warnings import warn
|
||
|
+ warn("The default separator of urllib.parse.parse_qsl and "
|
||
|
+ + "parse_qs was changed to '&' to avoid a web cache "
|
||
|
+ + "poisoning issue (CVE-2021-23336). "
|
||
|
+ + "By default, semicolons no longer act as query field "
|
||
|
+ + "separators. "
|
||
|
+ + "See https://access.redhat.com/articles/5860431 for "
|
||
|
+ + "more details.",
|
||
|
+ _QueryStringSeparatorWarning, stacklevel=2)
|
||
|
+ separator = '&'
|
||
|
+ elif separator == 'legacy':
|
||
|
+ separator = _legacy
|
||
|
+ elif len(separator) != 1:
|
||
|
+ raise ValueError(
|
||
|
+ f'{envvar_name} (from {config_source}) must contain '
|
||
|
+ + '1 character, or "legacy". See '
|
||
|
+ + 'https://access.redhat.com/articles/5860431 for more details.'
|
||
|
+ )
|
||
|
+
|
||
|
# If max_num_fields is defined then check that the number of fields
|
||
|
# is less than max_num_fields. This prevents a memory exhaustion DOS
|
||
|
# attack via post bodies with many fields.
|
||
|
if max_num_fields is not None:
|
||
|
- num_fields = 1 + qs.count('&') + qs.count(';')
|
||
|
+ if separator is _legacy:
|
||
|
+ num_fields = 1 + qs.count('&') + qs.count(';')
|
||
|
+ else:
|
||
|
+ num_fields = 1 + qs.count(separator)
|
||
|
if max_num_fields < num_fields:
|
||
|
raise ValueError('Max number of fields exceeded')
|
||
|
|
||
|
- pairs = [s2 for s1 in qs.split('&') for s2 in s1.split(';')]
|
||
|
+ if separator is _legacy:
|
||
|
+ pairs = [s2 for s1 in qs.split('&') for s2 in s1.split(';')]
|
||
|
+ else:
|
||
|
+ pairs = [s1 for s1 in qs.split(separator)]
|
||
|
r = []
|
||
|
for name_value in pairs:
|
||
|
if not name_value and not strict_parsing:
|
||
|
diff --git a/Misc/NEWS.d/next/Security/2021-02-14-15-59-16.bpo-42967.YApqDS.rst b/Misc/NEWS.d/next/Security/2021-02-14-15-59-16.bpo-42967.YApqDS.rst
|
||
|
new file mode 100644
|
||
|
index 00000000000..bc82c963067
|
||
|
--- /dev/null
|
||
|
+++ b/Misc/NEWS.d/next/Security/2021-02-14-15-59-16.bpo-42967.YApqDS.rst
|
||
|
@@ -0,0 +1 @@
|
||
|
+Make it possible to fix web cache poisoning vulnerability by allowing the user to choose a custom separator query args.
|