gh-102988: Detect email address parsing errors and return empty tuple to indicate...

Detect email address parsing errors and return empty tuple to indicate the parsing error (old API). This fixes or at least ameliorates CVE-2023-27043.

---------

Co-authored-by: Gregory P. Smith <greg@krypto.org>

diff --git a/Doc/library/email.utils.rst b/Doc/library/email.utils.rst
index 345b64001c1ace185ac37623604bddd68dcc7395..a87a0bd2e7de6bedc5b308dce77bd440f1137127 100644 (file)
--- a/Doc/library/email.utils.rst
+++ b/Doc/library/email.utils.rst
@@ -65,6 +65,11 @@ of the new API.
    *email address* parts.  Returns a tuple of that information, unless the parse
    fails, in which case a 2-tuple of ``('', '')`` is returned.
 
+   .. versionchanged:: 3.12
+      For security reasons, addresses that were ambiguous and could parse into
+      multiple different addresses now cause ``('', '')`` to be returned
+      instead of only one of the *potential* addresses.
+
 
 .. function:: formataddr(pair, charset='utf-8')
 
@@ -87,7 +92,7 @@ of the new API.
    This method returns a list of 2-tuples of the form returned by ``parseaddr()``.
    *fieldvalues* is a sequence of header field values as might be returned by
    :meth:`Message.get_all <email.message.Message.get_all>`.  Here's a simple
-   example that gets all the recipients of a message::
+   example that gets all the recipients of a message:
 
       from email.utils import getaddresses
 
@@ -97,6 +102,25 @@ of the new API.
       resent_ccs = msg.get_all('resent-cc', [])
       all_recipients = getaddresses(tos + ccs + resent_tos + resent_ccs)
 
+   When parsing fails for a single fieldvalue, a 2-tuple of ``('', '')``
+   is returned in its place.  Other errors in parsing the list of
+   addresses such as a fieldvalue seemingly parsing into multiple
+   addresses may result in a list containing a single empty 2-tuple
+   ``[('', '')]`` being returned rather than returning potentially
+   invalid output.
+
+   Example malformed input parsing:
+
+   .. doctest::
+
+      >>> from email.utils import getaddresses
+      >>> getaddresses(['alice@example.com <bob@example.com>', 'me@example.com'])
+      [('', '')]
+
+   .. versionchanged:: 3.12
+      The 2-tuple of ``('', '')`` in the returned values when parsing
+      fails were added as to address a security issue.
+
 
 .. function:: parsedate(date)
 

diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst
index a892f92dd26281d8d56cc8a86c1ef3569985fdfe..766f468cf9cfbceb677f45c1fa11836ab523863d 100644 (file)
--- a/Doc/whatsnew/3.12.rst
+++ b/Doc/whatsnew/3.12.rst
@@ -570,6 +570,14 @@ dis
   :data:`~dis.hasarg` collection instead.
   (Contributed by Irit Katriel in :gh:`94216`.)
 
+email
+-----
+
+* :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.
+  (Contributed by Thomas Dwyer for :gh:`102988` to ameliorate CVE-2023-27043.)
+
 fractions
 ---------
 

diff --git a/Lib/email/utils.py b/Lib/email/utils.py
index 81da5394ea169521d2c525f01480b1bf43b73c18..11ad75e94e9345935dcc87c7dede6f9eb8b670c0 100644 (file)
--- a/Lib/email/utils.py
+++ b/Lib/email/utils.py
@@ -106,12 +106,54 @@ def formataddr(pair, charset='utf-8'):
     return address
 
 
+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 getaddresses(fieldvalues):
-    """Return a list of (REALNAME, EMAIL) for each fieldvalue."""
-    all = COMMASPACE.join(str(v) for v in fieldvalues)
+    """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 the resulting list of parsed address is not the same as the number of
+    fieldvalues in the input list a parsing error has occurred.  A list
+    containing a single empty 2-tuple [('', '')] is returned in its place.
+    This is done to avoid invalid output.
+    """
+    fieldvalues = [str(v) for v in fieldvalues]
+    fieldvalues = _pre_parse_validation(fieldvalues)
+    all = COMMASPACE.join(v for v in fieldvalues)
     a = _AddressList(all)
-    return a.addresslist
+    result = _post_parse_validation(a.addresslist)
+
+    n = 0
+    for v in fieldvalues:
+        n += v.count(',') + 1
+
+    if len(result) != n:
+        return [('', '')]
+
+    return result
 
 
 def _format_timetuple_and_zone(timetuple, zone):
@@ -212,9 +254,18 @@ def parseaddr(addr):
     Return a tuple of realname and email address, unless the parse fails, in
     which case return a 2-tuple of ('', '').
     """
-    addrs = _AddressList(addr).addresslist
-    if not addrs:
-        return '', ''
+    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]
 
 

diff --git a/Lib/test/test_email/test_email.py b/Lib/test/test_email/test_email.py
index 44b405740c440313303d899d4e1cd5cff17fac92..5238944d6b47887c4e5e0acbbc56c230e5d2e7b1 100644 (file)
--- a/Lib/test/test_email/test_email.py
+++ b/Lib/test/test_email/test_email.py
@@ -3319,15 +3319,90 @@ Foo
            [('Al Person', 'aperson@dom.ain'),
             ('Bud Person', 'bperson@dom.ain')])
 
+    def test_getaddresses_parsing_errors(self):
+        """Test for parsing errors from CVE-2023-27043"""
+        eq = self.assertEqual
+        eq(utils.getaddresses(['alice@example.org(<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org)<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org<<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org><bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org@<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org,<bob@example.com>']),
+           [('', 'alice@example.org'), ('', 'bob@example.com')])
+        eq(utils.getaddresses(['alice@example.org;<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org:<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org.<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org"<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org[<bob@example.com>']),
+           [('', '')])
+        eq(utils.getaddresses(['alice@example.org]<bob@example.com>']),
+           [('', '')])
+
+    def test_parseaddr_parsing_errors(self):
+        """Test for parsing errors from CVE-2023-27043"""
+        eq = self.assertEqual
+        eq(utils.parseaddr(['alice@example.org(<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org)<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org<<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org><bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org@<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org,<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org;<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org:<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org.<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org"<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org[<bob@example.com>']),
+           ('', ''))
+        eq(utils.parseaddr(['alice@example.org]<bob@example.com>']),
+           ('', ''))
+
     def test_getaddresses_nasty(self):
         eq = self.assertEqual
         eq(utils.getaddresses(['foo: ;']), [('', '')])
-        eq(utils.getaddresses(
-           ['[]*-- =~$']),
-           [('', ''), ('', ''), ('', '*--')])
+        eq(utils.getaddresses(['[]*-- =~$']), [('', '')])
         eq(utils.getaddresses(
            ['foo: ;', '"Jason R. Mastaler" <jason@dom.ain>']),
            [('', ''), ('Jason R. Mastaler', 'jason@dom.ain')])
+        eq(utils.getaddresses(
+           [r'Pete(A nice \) chap) <pete(his account)@silly.test(his host)>']),
+           [('Pete (A nice ) chap his account his host)', 'pete@silly.test')])
+        eq(utils.getaddresses(
+           ['(Empty list)(start)Undisclosed recipients  :(nobody(I know))']),
+           [('', '')])
+        eq(utils.getaddresses(
+           ['Mary <@machine.tld:mary@example.net>, , jdoe@test   . example']),
+           [('Mary', 'mary@example.net'), ('', ''), ('', 'jdoe@test.example')])
+        eq(utils.getaddresses(
+           ['John Doe <jdoe@machine(comment).  example>']),
+           [('John Doe (comment)', 'jdoe@machine.example')])
+        eq(utils.getaddresses(
+           ['"Mary Smith: Personal Account" <smith@home.example>']),
+           [('Mary Smith: Personal Account', 'smith@home.example')])
+        eq(utils.getaddresses(
+           ['Undisclosed recipients:;']),
+           [('', '')])
+        eq(utils.getaddresses(
+           [r'<boss@nil.test>, "Giant; \"Big\" Box" <bob@example.net>']),
+           [('', 'boss@nil.test'), ('Giant; "Big" Box', 'bob@example.net')])
 
     def test_getaddresses_embedded_comment(self):
         """Test proper handling of a nested comment"""

diff --git a/Misc/NEWS.d/next/Security/2023-06-13-20-52-24.gh-issue-102988.Kei7Vf.rst b/Misc/NEWS.d/next/Security/2023-06-13-20-52-24.gh-issue-102988.Kei7Vf.rst
new file mode 100644 (file)
index 0000000..e0434cc
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2023-06-13-20-52-24.gh-issue-102988.Kei7Vf.rst
@@ -0,0 +1,4 @@
+CVE-2023-27043: Prevent :func:`email.utils.parseaddr`
+and :func:`email.utils.getaddresses` from returning the realname portion of an
+invalid RFC2822 email header in the email address portion of the 2-tuple
+returned after being parsed by :class:`email._parseaddr.AddressList`.