forked from python/cpython
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
pythongh-102988: email parseaddr() now rejects malformed address
Detect email address parsing errors and return empty tuple to indicate the parsing error (old API). Add an optional 'strict' parameter to getaddresses() and parseaddr() functions. Patch by Thomas Dwyer. Co-Authored-By: Thomas Dwyer <[email protected]>
- Loading branch information
Showing
5 changed files
with
252 additions
and
36 deletions.
There are no files selected for viewing
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
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
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 |
---|---|---|
|
@@ -42,6 +42,8 @@ | |
|
||
specialsre = re.compile(r'[][\\()<>@,:;".]') | ||
escapesre = re.compile(r'[\\"]') | ||
realname_comma_re = re.compile(r'"[^",]*,[^"]*"') | ||
|
||
|
||
def _has_surrogates(s): | ||
"""Return True if s contains surrogate-escaped binary data.""" | ||
|
@@ -103,12 +105,97 @@ def formataddr(pair, charset='utf-8'): | |
return address | ||
|
||
|
||
def _strip_quoted_realname(addr): | ||
"""Remove real name between quotes.""" | ||
pos = 0 | ||
start = None | ||
remove = [] | ||
while pos < len(addr): | ||
if addr[pos] == '"': | ||
if start is None: | ||
start = pos | ||
else: | ||
remove.append((start, pos)) | ||
start = None | ||
pos += 1 | ||
|
||
if not remove: | ||
return addr | ||
|
||
result = [] | ||
pos = 0 | ||
for start, stop in remove: | ||
result.append(addr[pos:start]) | ||
pos = stop + 1 | ||
result.append(addr[pos:]) | ||
|
||
return ''.join(result) | ||
|
||
def getaddresses(fieldvalues): | ||
"""Return a list of (REALNAME, EMAIL) for each fieldvalue.""" | ||
all = COMMASPACE.join(str(v) for v in fieldvalues) | ||
a = _AddressList(all) | ||
return a.addresslist | ||
|
||
def getaddresses(fieldvalues, *, strict=True): | ||
"""Return a list of (REALNAME, EMAIL) or ('','') for each fieldvalue. | ||
When parsing fails for a fieldvalue, a 2-tuple of ('', '') is returned in | ||
its place. | ||
If strict is True, use a strict parser which rejects malformed inputs. | ||
In this case, if the resulting list of parsed addresses is greater than | ||
the number of fieldvalues in the input list, a parsing error has occurred | ||
and consequently a list containing a single empty 2-tuple [('', '')] is | ||
returned in its place. This is done to avoid invalid output. | ||
Malformed input: getaddresses(['[email protected] <[email protected]>']) | ||
Invalid output: [('', '[email protected]'), ('', '[email protected]')] | ||
Safe output: [('', '')] | ||
""" | ||
if not strict: | ||
all = COMMASPACE.join(str(v) for v in fieldvalues) | ||
a = _AddressList(all) | ||
return a.addresslist | ||
|
||
fieldvalues = [str(v) for v in fieldvalues] | ||
fieldvalues = _pre_parse_validation(fieldvalues) | ||
addr = COMMASPACE.join(v for v in fieldvalues) | ||
a = _AddressList(addr) | ||
result = _post_parse_validation(a.addresslist) | ||
|
||
# Treat output as invalid if the number of addresses is not equal to the | ||
# expected number of addresses. | ||
n = 0 | ||
for v in fieldvalues: | ||
# When a comma is used in the Real Name part it is not a deliminator. | ||
# So strip those out before counting the commas | ||
v = _strip_quoted_realname(v) | ||
# Expected number of addresses: 1 + number of commas | ||
n += 1 + v.count(',') | ||
if len(result) != n: | ||
return [('', '')] | ||
|
||
return result | ||
|
||
|
||
def _pre_parse_validation(email_header_fields): | ||
accepted_values = [] | ||
for v in email_header_fields: | ||
s = v.replace('\\(', '').replace('\\)', '') | ||
if s.count('(') != s.count(')'): | ||
v = "('', '')" | ||
accepted_values.append(v) | ||
|
||
return accepted_values | ||
|
||
|
||
def _post_parse_validation(parsed_email_header_tuples): | ||
accepted_values = [] | ||
# The parser would have parsed a correctly formatted domain-literal | ||
# The existence of an [ after parsing indicates a parsing failure | ||
for v in parsed_email_header_tuples: | ||
if '[' in v[1]: | ||
v = ('', '') | ||
accepted_values.append(v) | ||
|
||
return accepted_values | ||
|
||
|
||
def _format_timetuple_and_zone(timetuple, zone): | ||
|
@@ -207,16 +294,33 @@ def parsedate_to_datetime(data): | |
tzinfo=datetime.timezone(datetime.timedelta(seconds=tz))) | ||
|
||
|
||
def parseaddr(addr): | ||
def parseaddr(addr, *, strict=True): | ||
""" | ||
Parse addr into its constituent realname and email address parts. | ||
Return a tuple of realname and email address, unless the parse fails, in | ||
which case return a 2-tuple of ('', ''). | ||
If strict is True, use a strict parser which rejects malformed inputs. | ||
""" | ||
addrs = _AddressList(addr).addresslist | ||
if not addrs: | ||
return '', '' | ||
if not strict: | ||
addrs = _AddressList(addr).addresslist | ||
if not addrs: | ||
return ('', '') | ||
return addrs[0] | ||
|
||
if isinstance(addr, list): | ||
addr = addr[0] | ||
|
||
if not isinstance(addr, str): | ||
return ('', '') | ||
|
||
addr = _pre_parse_validation([addr])[0] | ||
addrs = _post_parse_validation(_AddressList(addr).addresslist) | ||
|
||
if not addrs or len(addrs) > 1: | ||
return ('', '') | ||
|
||
return addrs[0] | ||
|
||
|
||
|
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 |
---|---|---|
|
@@ -3320,32 +3320,116 @@ def test_getaddresses(self): | |
[('Al Person', '[email protected]'), | ||
('Bud Person', '[email protected]')]) | ||
|
||
def test_getaddresses_comma_in_name(self): | ||
"""GH-106669 regression test.""" | ||
self.assertEqual( | ||
utils.getaddresses( | ||
[ | ||
'"Bud, Person" <[email protected]>', | ||
'[email protected] (Al Person)', | ||
'"Mariusz Felisiak" <[email protected]>', | ||
] | ||
), | ||
[ | ||
('Bud, Person', '[email protected]'), | ||
('Al Person', '[email protected]'), | ||
('Mariusz Felisiak', '[email protected]'), | ||
], | ||
) | ||
def test_parsing_errors(self): | ||
"""Test for parsing errors from CVE-2023-27043 and CVE-2019-16056""" | ||
alice = '[email protected]' | ||
bob = '[email protected]' | ||
empty = ('', '') | ||
|
||
# Test utils.getaddresses() and utils.parseaddr() on malformed email | ||
# addresses: default behavior (strict=True) rejects malformed address, | ||
# and strict=True which tolerates malformed address. | ||
for invalid_separator, expected_non_strict in ( | ||
('(', [(f'<{bob}>', alice)]), | ||
(')', [('', alice), empty, ('', bob)]), | ||
('<', [('', alice), empty, ('', bob), empty]), | ||
('>', [('', alice), empty, ('', bob)]), | ||
('[', [('', f'{alice}[<{bob}>]')]), | ||
(']', [('', alice), empty, ('', bob)]), | ||
('@', [empty, empty, ('', bob)]), | ||
(';', [('', alice), empty, ('', bob)]), | ||
(':', [('', alice), ('', bob)]), | ||
('.', [('', alice + '.'), ('', bob)]), | ||
('"', [('', alice), ('', f'<{bob}>')]), | ||
): | ||
address = f'{alice}{invalid_separator}<{bob}>' | ||
with self.subTest(address=address): | ||
self.assertEqual(utils.getaddresses([address]), | ||
[empty]) | ||
self.assertEqual(utils.getaddresses([address], strict=False), | ||
expected_non_strict) | ||
|
||
self.assertEqual(utils.parseaddr([address]), | ||
empty) | ||
self.assertEqual(utils.parseaddr([address], strict=False), | ||
('', address)) | ||
|
||
# Comma (',') is treated differently depending on strict parameter. | ||
# Comma without quotes. | ||
address = f'{alice},<{bob}>' | ||
self.assertEqual(utils.getaddresses([address]), | ||
[('', alice), ('', bob)]) | ||
self.assertEqual(utils.getaddresses([address], strict=False), | ||
[('', alice), ('', bob)]) | ||
self.assertEqual(utils.parseaddr([address]), | ||
empty) | ||
self.assertEqual(utils.parseaddr([address], strict=False), | ||
('', address)) | ||
|
||
# Comma with quotes. | ||
address = '"Alice, [email protected]" <[email protected]>' | ||
expected_strict = ('Alice, [email protected]', '[email protected]') | ||
self.assertEqual(utils.getaddresses([address]), [expected_strict]) | ||
self.assertEqual(utils.getaddresses([address], strict=False), [expected_strict]) | ||
self.assertEqual(utils.parseaddr([address]), expected_strict) | ||
self.assertEqual(utils.parseaddr([address], strict=False), | ||
('', address)) | ||
|
||
# Two addresses with quotes separated by comma. | ||
address = '"Jane Doe" <[email protected]>, "John Doe" <[email protected]>' | ||
self.assertEqual(utils.getaddresses([address]), | ||
[('Jane Doe', '[email protected]'), | ||
('John Doe', '[email protected]')]) | ||
self.assertEqual(utils.getaddresses([address], strict=False), | ||
[('Jane Doe', '[email protected]'), | ||
('John Doe', '[email protected]')]) | ||
self.assertEqual(utils.parseaddr([address]), empty) | ||
self.assertEqual(utils.parseaddr([address], strict=False), | ||
('', address)) | ||
|
||
def test_getaddresses_nasty(self): | ||
eq = self.assertEqual | ||
eq(utils.getaddresses(['foo: ;']), [('', '')]) | ||
eq(utils.getaddresses( | ||
['[]*-- =~$']), | ||
[('', ''), ('', ''), ('', '*--')]) | ||
eq(utils.getaddresses( | ||
['foo: ;', '"Jason R. Mastaler" <[email protected]>']), | ||
[('', ''), ('Jason R. Mastaler', '[email protected]')]) | ||
for addresses, expected in ( | ||
(['"Sürname, Firstname" <[email protected]>'], | ||
[('Sürname, Firstname', '[email protected]')]), | ||
|
||
(['foo: ;'], | ||
[('', '')]), | ||
|
||
(['foo: ;', '"Jason R. Mastaler" <[email protected]>'], | ||
[('', ''), ('Jason R. Mastaler', '[email protected]')]), | ||
|
||
([r'Pete(A nice \) chap) <pete(his account)@silly.test(his host)>'], | ||
[('Pete (A nice ) chap his account his host)', '[email protected]')]), | ||
|
||
(['(Empty list)(start)Undisclosed recipients :(nobody(I know))'], | ||
[('', '')]), | ||
|
||
(['Mary <@machine.tld:[email protected]>, , jdoe@test . example'], | ||
[('Mary', '[email protected]'), ('', ''), ('', '[email protected]')]), | ||
|
||
(['John Doe <jdoe@machine(comment). example>'], | ||
[('John Doe (comment)', '[email protected]')]), | ||
|
||
(['"Mary Smith: Personal Account" <[email protected]>'], | ||
[('Mary Smith: Personal Account', '[email protected]')]), | ||
|
||
(['Undisclosed recipients:;'], | ||
[('', '')]), | ||
|
||
([r'<[email protected]>, "Giant; \"Big\" Box" <[email protected]>'], | ||
[('', '[email protected]'), ('Giant; "Big" Box', '[email protected]')]), | ||
): | ||
with self.subTest(addresses=addresses): | ||
self.assertEqual(utils.getaddresses(addresses), | ||
expected) | ||
self.assertEqual(utils.getaddresses(addresses, strict=False), | ||
expected) | ||
|
||
addresses = ['[]*-- =~$'] | ||
self.assertEqual(utils.getaddresses(addresses), | ||
[('', '')]) | ||
self.assertEqual(utils.getaddresses(addresses, strict=False), | ||
[('', ''), ('', ''), ('', '*--')]) | ||
|
||
def test_getaddresses_embedded_comment(self): | ||
"""Test proper handling of a nested comment""" | ||
|
6 changes: 6 additions & 0 deletions
6
Misc/NEWS.d/next/Library/2023-10-20-15-28-08.gh-issue-102988.dStNO7.rst
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,6 @@ | ||
:func:`email.utils.getaddresses` and :func:`email.utils.parseaddr` now | ||
return ``('', '')`` 2-tuples in more situations where invalid email | ||
addresses are encountered instead of potentially inaccurate values. Add | ||
optional *strict* parameter to these two functions: use ``strict=False`` to | ||
get the old behavior, accept malformed inputs. Patch by Thomas Dwyer to | ||
improve the CVE-2023-27043 fix. |