-
-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #118403 from dotlambda/CVE-2021-23336
python27: fix CVE-2021-23336
- Loading branch information
Showing
2 changed files
with
392 additions
and
0 deletions.
There are no files selected for viewing
390 changes: 390 additions & 0 deletions
390
pkgs/development/interpreters/python/cpython/2.7/CVE-2021-23336.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,390 @@ | ||
From e7b005c05dbdbce967a409abd71641281a8604bf Mon Sep 17 00:00:00 2001 | ||
From: Senthil Kumaran <[email protected]> | ||
Date: Mon, 15 Feb 2021 11:16:43 -0800 | ||
Subject: [PATCH 24/26] [3.6] bpo-42967: only use '&' as a query string | ||
separator (GH-24297) (GH-24532) | ||
MIME-Version: 1.0 | ||
Content-Type: text/plain; charset=UTF-8 | ||
Content-Transfer-Encoding: 8bit | ||
|
||
bpo-42967: [security] Address a web cache-poisoning issue reported in | ||
urllib.parse.parse_qsl(). | ||
|
||
urllib.parse will only us "&" as query string separator by default | ||
instead of both ";" and "&" as allowed in earlier versions. An optional | ||
argument seperator with default value "&" is added to specify the | ||
separator. | ||
|
||
Co-authored-by: Éric Araujo <[email protected]> | ||
Co-authored-by: Ken Jin <[email protected]> | ||
Co-authored-by: Adam Goldschmidt <[email protected]> | ||
|
||
Rebased for Python 2.7 by Michał Górny | ||
--- | ||
Doc/library/cgi.rst | 7 +++- | ||
Doc/library/urlparse.rst | 23 ++++++++++- | ||
Lib/cgi.py | 20 +++++++--- | ||
Lib/test/test_cgi.py | 29 +++++++++++--- | ||
Lib/test/test_urlparse.py | 38 +++++++++---------- | ||
Lib/urlparse.py | 22 ++++++++--- | ||
.../2021-02-14-15-59-16.bpo-42967.YApqDS.rst | 1 + | ||
7 files changed, 100 insertions(+), 40 deletions(-) | ||
create mode 100644 Misc/NEWS.d/next/Security/2021-02-14-15-59-16.bpo-42967.YApqDS.rst | ||
|
||
diff --git a/Doc/library/cgi.rst b/Doc/library/cgi.rst | ||
index ecd62c8c01..b85cdd8b61 100644 | ||
--- a/Doc/library/cgi.rst | ||
+++ b/Doc/library/cgi.rst | ||
@@ -285,10 +285,10 @@ 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[, environ[, keep_blank_values[, strict_parsing]]]) | ||
+.. function:: parse(fp[, environ[, keep_blank_values[, strict_parsing]]], separator="&") | ||
|
||
Parse a query in the environment or from a file (the file defaults to | ||
- ``sys.stdin`` and environment defaults to ``os.environ``). The *keep_blank_values* and *strict_parsing* parameters are | ||
+ ``sys.stdin`` and environment defaults to ``os.environ``). The *keep_blank_values*, *strict_parsing* and *separator* parameters are | ||
passed to :func:`urlparse.parse_qs` unchanged. | ||
|
||
|
||
@@ -316,6 +316,9 @@ algorithms implemented in this module in other circumstances. | ||
Note that this does not parse nested multipart parts --- use | ||
:class:`FieldStorage` for that. | ||
|
||
+ .. versionchanged:: 3.6.13 | ||
+ Added the *separator* parameter. | ||
+ | ||
|
||
.. function:: parse_header(string) | ||
|
||
diff --git a/Doc/library/urlparse.rst b/Doc/library/urlparse.rst | ||
index 0989c88c30..2f8e4c5a44 100644 | ||
--- a/Doc/library/urlparse.rst | ||
+++ b/Doc/library/urlparse.rst | ||
@@ -136,7 +136,7 @@ The :mod:`urlparse` module defines the following functions: | ||
now raise :exc:`ValueError`. | ||
|
||
|
||
-.. function:: parse_qs(qs[, keep_blank_values[, strict_parsing[, max_num_fields]]]) | ||
+.. function:: parse_qs(qs[, keep_blank_values[, strict_parsing[, max_num_fields]]], separator='&') | ||
|
||
Parse a query string given as a string argument (data of type | ||
:mimetype:`application/x-www-form-urlencoded`). Data are returned as a | ||
@@ -157,6 +157,9 @@ The :mod:`urlparse` module defines the following functions: | ||
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 defaults to ``&``. | ||
+ | ||
Use the :func:`urllib.urlencode` function to convert such dictionaries into | ||
query strings. | ||
|
||
@@ -166,7 +169,14 @@ The :mod:`urlparse` module defines the following functions: | ||
.. versionchanged:: 2.7.16 | ||
Added *max_num_fields* parameter. | ||
|
||
-.. function:: parse_qsl(qs[, keep_blank_values[, strict_parsing[, max_num_fields]]]) | ||
+ .. versionchanged:: 2.7.18-gentoo | ||
+ Added *separator* parameter with the default value of ``&``. Earlier | ||
+ Python versions allowed using both ``;`` and ``&`` as query parameter | ||
+ separator. This has been changed to allow only a single separator key, | ||
+ with ``&`` as the default separator. | ||
+ | ||
+ | ||
+.. function:: parse_qsl(qs[, keep_blank_values[, strict_parsing[, max_num_fields]]], separator='&') | ||
|
||
Parse a query string given as a string argument (data of type | ||
:mimetype:`application/x-www-form-urlencoded`). Data are returned as a list of | ||
@@ -186,6 +196,9 @@ The :mod:`urlparse` module defines the following functions: | ||
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 defaults to ``&``. | ||
+ | ||
Use the :func:`urllib.urlencode` function to convert such lists of pairs into | ||
query strings. | ||
|
||
@@ -195,6 +208,12 @@ The :mod:`urlparse` module defines the following functions: | ||
.. versionchanged:: 2.7.16 | ||
Added *max_num_fields* parameter. | ||
|
||
+ .. versionchanged:: 2.7.18-gentoo | ||
+ Added *separator* parameter with the default value of ``&``. Earlier | ||
+ Python versions allowed using both ``;`` and ``&`` as query parameter | ||
+ separator. This has been changed to allow only a single separator key, | ||
+ with ``&`` as the default separator. | ||
+ | ||
.. function:: urlunparse(parts) | ||
|
||
Construct a URL from a tuple as returned by ``urlparse()``. The *parts* argument | ||
diff --git a/Lib/cgi.py b/Lib/cgi.py | ||
index 5b903e0347..9d0848b6b1 100755 | ||
--- a/Lib/cgi.py | ||
+++ b/Lib/cgi.py | ||
@@ -121,7 +121,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='&'): | ||
"""Parse a query in the environment or from a file (default stdin) | ||
|
||
Arguments, all optional: | ||
@@ -140,6 +141,9 @@ 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. | ||
+ Defaults to &. | ||
""" | ||
if fp is None: | ||
fp = sys.stdin | ||
@@ -171,7 +175,8 @@ def parse(fp=None, environ=os.environ, keep_blank_values=0, strict_parsing=0): | ||
else: | ||
qs = "" | ||
environ['QUERY_STRING'] = qs # XXX Shouldn't, really | ||
- return urlparse.parse_qs(qs, keep_blank_values, strict_parsing) | ||
+ return urlparse.parse_qs(qs, keep_blank_values, strict_parsing, | ||
+ separator=separator) | ||
|
||
|
||
# parse query string function called from urlparse, | ||
@@ -395,7 +400,7 @@ class FieldStorage: | ||
|
||
def __init__(self, fp=None, headers=None, outerboundary="", | ||
environ=os.environ, keep_blank_values=0, strict_parsing=0, | ||
- max_num_fields=None): | ||
+ max_num_fields=None, separator='&'): | ||
"""Constructor. Read multipart/* until last part. | ||
|
||
Arguments, all optional: | ||
@@ -430,6 +435,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 | ||
@@ -613,7 +619,8 @@ class FieldStorage: | ||
if self.qs_on_post: | ||
qs += '&' + self.qs_on_post | ||
query = urlparse.parse_qsl(qs, self.keep_blank_values, | ||
- self.strict_parsing, self.max_num_fields) | ||
+ self.strict_parsing, self.max_num_fields, | ||
+ separator=self.separator) | ||
self.list = [MiniFieldStorage(key, value) for key, value in query] | ||
self.skip_lines() | ||
|
||
@@ -629,7 +636,8 @@ class FieldStorage: | ||
query = urlparse.parse_qsl(self.qs_on_post, | ||
self.keep_blank_values, | ||
self.strict_parsing, | ||
- self.max_num_fields) | ||
+ self.max_num_fields, | ||
+ separator=self.separator) | ||
self.list.extend(MiniFieldStorage(key, value) | ||
for key, value in query) | ||
FieldStorageClass = None | ||
@@ -649,7 +657,7 @@ class FieldStorage: | ||
headers = rfc822.Message(self.fp) | ||
part = klass(self.fp, headers, ib, | ||
environ, keep_blank_values, strict_parsing, | ||
- max_num_fields) | ||
+ 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 743c2afbd4..f414faa23b 100644 | ||
--- a/Lib/test/test_cgi.py | ||
+++ b/Lib/test/test_cgi.py | ||
@@ -61,12 +61,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: ''")), | ||
@@ -81,8 +78,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'], | ||
@@ -188,6 +183,30 @@ class CgiTests(unittest.TestCase): | ||
self.assertEqual(expect[k], v) | ||
self.assertItemsEqual(expect.values(), d.values()) | ||
|
||
+ 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): | ||
+ 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_log(self): | ||
cgi.log("Testing") | ||
|
||
diff --git a/Lib/test/test_urlparse.py b/Lib/test/test_urlparse.py | ||
index 86c4a0595c..0b2107339a 100644 | ||
--- a/Lib/test/test_urlparse.py | ||
+++ b/Lib/test/test_urlparse.py | ||
@@ -24,16 +24,20 @@ parse_qsl_test_cases = [ | ||
("&a=b", [('a', 'b')]), | ||
("a=a+b&b=b+c", [('a', 'a b'), ('b', 'b c')]), | ||
("a=1&a=2", [('a', '1'), ('a', '2')]), | ||
- (";", []), | ||
- (";;", []), | ||
- (";a=b", [('a', 'b')]), | ||
- ("a=a+b;b=b+c", [('a', 'a b'), ('b', 'b c')]), | ||
- ("a=1;a=2", [('a', '1'), ('a', '2')]), | ||
- (b";", []), | ||
- (b";;", []), | ||
- (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')]), | ||
+ (b"", []), | ||
+ (b"&", []), | ||
+ (b"&&", []), | ||
+ (b"=", [(b'', b'')]), | ||
+ (b"=a", [(b'', b'a')]), | ||
+ (b"a", [(b'a', b'')]), | ||
+ (b"a=", [(b'a', b'')]), | ||
+ (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')]), | ||
+ (";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')]), | ||
] | ||
|
||
parse_qs_test_cases = [ | ||
@@ -57,16 +61,10 @@ 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']}), | ||
- (";", {}), | ||
- (";;", {}), | ||
- (";a=b", {'a': ['b']}), | ||
- ("a=a+b;b=b+c", {'a': ['a b'], 'b': ['b c']}), | ||
- ("a=1;a=2", {'a': ['1', '2']}), | ||
- (b";", {}), | ||
- (b";;", {}), | ||
- (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']}), | ||
+ (";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']}), | ||
] | ||
|
||
class UrlParseTestCase(unittest.TestCase): | ||
diff --git a/Lib/urlparse.py b/Lib/urlparse.py | ||
index 798b467b60..6c32727fce 100644 | ||
--- a/Lib/urlparse.py | ||
+++ b/Lib/urlparse.py | ||
@@ -382,7 +382,8 @@ def unquote(s): | ||
append(item) | ||
return ''.join(res) | ||
|
||
-def parse_qs(qs, keep_blank_values=0, strict_parsing=0, max_num_fields=None): | ||
+def parse_qs(qs, keep_blank_values=0, strict_parsing=0, max_num_fields=None, | ||
+ separator='&'): | ||
"""Parse a query given as a string argument. | ||
|
||
Arguments: | ||
@@ -402,17 +403,22 @@ def parse_qs(qs, keep_blank_values=0, strict_parsing=0, max_num_fields=None): | ||
|
||
max_num_fields: int. If set, then throws a ValueError if there | ||
are more than n fields read by parse_qsl(). | ||
+ | ||
+ separator: str. The symbol to use for separating the query arguments. | ||
+ Defaults to &. | ||
+ | ||
""" | ||
dict = {} | ||
for name, value in parse_qsl(qs, keep_blank_values, strict_parsing, | ||
- max_num_fields): | ||
+ max_num_fields, separator=separator): | ||
if name in dict: | ||
dict[name].append(value) | ||
else: | ||
dict[name] = [value] | ||
return dict | ||
|
||
-def parse_qsl(qs, keep_blank_values=0, strict_parsing=0, max_num_fields=None): | ||
+def parse_qsl(qs, keep_blank_values=0, strict_parsing=0, max_num_fields=None, | ||
+ separator='&'): | ||
"""Parse a query given as a string argument. | ||
|
||
Arguments: | ||
@@ -432,17 +438,23 @@ def parse_qsl(qs, keep_blank_values=0, strict_parsing=0, max_num_fields=None): | ||
max_num_fields: int. If set, then throws a ValueError if there | ||
are more than n fields read by parse_qsl(). | ||
|
||
+ separator: str. The symbol to use for separating the query arguments. | ||
+ Defaults to &. | ||
+ | ||
Returns a list, as G-d intended. | ||
""" | ||
+ if not separator or (not isinstance(separator, (str, bytes))): | ||
+ raise ValueError("Separator must be of type string or bytes.") | ||
+ | ||
# 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(';') | ||
+ 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(';')] | ||
+ 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 0000000000..f08489b414 | ||
--- /dev/null | ||
+++ b/Misc/NEWS.d/next/Security/2021-02-14-15-59-16.bpo-42967.YApqDS.rst | ||
@@ -0,0 +1 @@ | ||
+Fix web cache poisoning vulnerability by defaulting the query args separator to ``&``, and allowing the user to choose a custom separator. | ||
-- | ||
2.31.1 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters