GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — [CVE-2019-16056] email.utils.parseaddr mistakenly parse an email · Issue #78336 · python/cpython

[CVE-2019-16056] email.utils.parseaddr mistakenly parse an email #78336

New issue

Copy link

New issue

Copy link

Closed

[CVE-2019-16056] email.utils.parseaddr mistakenly parse an email#78336

Copy link

Labels

3.7 (EOL)end of lifeend of life3.8 (EOL)end of lifeend of life3.9 (EOL)end of lifeend of lifetopic-emailtype-securityA security issueA security issue

Description

cnicodeme

mannequin

opened

on Jul 19, 2018

Issue body actions

BPO	34155
Nosy	@warsaw, @vstinner, @msapiro, @larryhastings, @ned-deily, @mcepl, @bitdancer, @ambv, @nicoe, @maxking, @ksze, @miss-islington, @Windsooon, @tirkarthi, @cnicodeme, @jpic, @ret2libc, @aeros, @rcsanchez97
PRs	bpo-34155: Dont parse domains containing @ #13079 [3.8] bpo-34155: Dont parse domains containing @ (GH-13079) #14824 [3.7] bpo-34155: Dont parse domains containing @ (GH-13079) #14825 [3.6] bpo-34155: Dont parse domains containing @ (GH-13079) #14826 [3.5] bpo-34155: Dont parse domains containing @ (GH-13079) #15317 [2.7] bpo-34155: Dont parse domains containing @ (GH-13079) #16006
Files	Screen Shot 2019-05-02 at 22.07.27.png

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-09-14.17:43:40.610>
created_at = <Date 2018-07-19.14:53:43.774>
labels = ['type-security', '3.7', '3.8', 'expert-email', '3.9']
title = '[CVE-2019-16056] email.utils.parseaddr mistakenly parse an email'
updated_at = <Date 2019-09-14.17:43:40.609>
user = 'https://github.com/cnicodeme'

bugs.python.org fields:

activity = <Date 2019-09-14.17:43:40.609>
actor = 'maxking'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-14.17:43:40.610>
closer = 'maxking'
components = ['email']
creation = <Date 2018-07-19.14:53:43.774>
creator = 'cnicodeme'
dependencies = []
files = ['48295']
hgrepos = []
issue_num = 34155
keywords = ['patch', 'security_issue']
message_count = 52.0
messages = ['321956', '321957', '321958', '321959', '321967', '329372', '329376', '329377', '329379', '329380', '329382', '329463', '340534', '340535', '340933', '341069', '341294', '341320', '341322', '341362', '341367', '341370', '341381', '344030', '344157', '344205', '344389', '344431', '344432', '347157', '347183', '347223', '348082', '349278', '349279', '349292', '349357', '349464', '349465', '349820', '349891', '349892', '349957', '349968', '350291', '351281', '351283', '351364', '351377', '352230', '352444', '352445']
nosy_count = 22.0
nosy_names = ['barry', 'vstinner', 'msapiro', 'larry', 'ned.deily', 'mcepl', 'r.david.murray', 'lukasz.langa', 'nicoe', 'maxking', 'kal.sze', 'miss-islington', 'Windson Yang', 'xtreak', 'cnicodeme', 'bortzmeyer', 'jpic', 'Dain Dwarf', 'rschiron', 'aeros', 'Anselmo Melo', 'rcsanchez97']
pr_nums = ['13079', '14824', '14825', '14826', '15317', '16006']
priority = 'critical'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue34155'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9']

Activity

cnicodeme commented on Jul 19, 2018
cnicodememannequin
on Jul 19, 2018
MannequinAuthor
More actions
Hi!

I'm trying to parse some emails, and I discovered that email.utils.parseaddr wrongly parse an email.

Here's the corresponding header:

From: =?utf-8?Q?[email protected]=E3=82=86=E2=86=91=E3=82=86?=
=?utf-8?Q?=E3=82=83=E3=82=85=E3=81=87=E3=81=BA=E3=81=BD=E3=81=BC"\=E3?=
=?utf-8?Q?=81=A9=E3=81=A5=E3=81=A2l=E3=81=A0=E3=81=B0=E3=81=A8=E3=81?=
=?utf-8?Q?=8FKL=E3=81=84=E3=82=8C=E3=82=8B=E3=82=86>KL=E3=82=89JF?=
<[email protected]>

Once this has been parsed via decode_header, we obtain this value:

From: [email protected]ゆ↑ゆゃゅぇぺぽぼ"\どづぢlだばとくKLいれるゆ>KLらJF <[email protected]>

(I agree, not really a nice looking From email ...)

Then, when this value is given to parseaddr, here's the result:

('', '[email protected]ゆ↑ゆゃゅぇぺぽぼ')

But it should be:

('[email protected]ゆ↑ゆゃゅぇぺぽぼ"\どづぢlだばとくKLいれるゆ>KLらJF', '[email protected]')

(Note that the email in the "name" part is not the same as the email in the "email" part!)
cnicodeme
added
type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

topic-email
on Jul 19, 2018

bitdancer commented on Jul 19, 2018

bitdancer

on Jul 19, 2018

Member

More actions

That does appear to be a bug. Note that the new email API handles it correctly:

    >>> x = """
    ... > From: [email protected]=E3=82=86=E2=86=91=E3=82=86?=
    ...  =?utf-8?Q?=E3=82=83=E3=82=85=E3=81=87=E3=81=BA=E3=81=BD=E3=81=BC"\=E3?=
    ...  =?utf-8?Q?=81=A9=E3=81=A5=E3=81=A2l=E3=81=A0=E3=81=B0=E3=81=A8=E3=81?=
    ...  =?utf-8?Q?=8FKL=E3=81=84=E3=82=8C=E3=82=8B=E3=82=86>KL=E3=82=89JF?=
    ...  <[email protected]>
    ... """
    >>> from email import message_from_string
    >>> from email.policy import default
    >>> m = message_from_string(x+'\n\ntest', policy=default)
    >>> m['from']
    '"[email protected]ゆ↑ゆ ゃゅぇぺぽぼ\\"\\\\� ��づぢlだばと� �KLいれるゆ>KLらJF" <[email protected]>'
    >>> m['from'].addresses[0].addr_spec
    '[email protected]'
    >>> m['from'].addresses[0].display_name
    '[email protected]ゆ↑ゆ ゃゅぇぺぽぼ"\\\udce3 \udc81\udca9づぢlだばと\udce3\udc81 \udc8fKLいれるゆ>KLらJF'

I'm not particularly interested myself in fixing parseaddr to handle this case correctly, since it is the legacy API, but if someone else wants to I'll review the patch.

bitdancer
added
3.7 (EOL)end of lifeend of life

3.8 (EOL)end of lifeend of life
on Jul 19, 2018
bitdancer commented on Jul 19, 2018
bitdancer
on Jul 19, 2018
Member
More actions
Oops, I left out a step in that cut and paste. For completeness:

>>> x = x[3:]
bitdancer commented on Jul 19, 2018
bitdancer
on Jul 19, 2018
Member
More actions
Ah, maybe it doesn't handle it completely correctly; that decode looks different now that I look at it in detail.
jwilk commented on Jul 19, 2018
jwilkmannequin
on Jul 19, 2018
Mannequin
More actions
You should not use decode_header() on the whole From header, because that loses
information. You should parse the header first, then decode the parts that
could be RFC2047-encoded.

Quoting <https://tools.ietf.org/html/rfc2047#section-6.2\>:

NOTE: Decoding and display of encoded-words occurs *after* a
structured field body is parsed into tokens. It is therefore
possible to hide 'special' characters in encoded-words which, when
displayed, will be indistinguishable from 'special' characters in the
surrounding text. For this and other reasons, it is NOT generally
possible to translate a message header containing 'encoded-word's to
an unencoded form which can be parsed by an RFC 822 mail reader.

So I don't see a bug in parseaddr() here, except that the API is a bit of a
footgun.
msapiro commented on Nov 6, 2018
msapiromannequin
on Nov 6, 2018
Mannequin
More actions
The issue is illustrated much more simply as follows:

email.utils.parseaddr('John Doe [email protected] <[email protected]>')

returns

('', 'John Doe [email protected]')

whereas it should return

('John Doe [email protected]', '[email protected]')

I'll look at developing a patch.
bitdancer commented on Nov 6, 2018
bitdancer
on Nov 6, 2018
Member
More actions

>> m = message_from_string("From: John Doe [email protected] <[email protected]>\n\n", policy=default)
>>> m['From'].addresses(Address(display_name='', username='John Doe jdoe', domain='example.com'),)

The new policies have more error recovery for non-RFC compliant addresses than decode_header, but the two agree in this case. What is happening here is that (1) an unquoted/unencoded '@' is not allowed in a display name (2) if the address is not '<>' quoted, then everything before the @ is the username and (3) in the absence of a comma after the end of the fqdn (which is not allowed to contain blanks) any additional tokens are discarded.

One could argue that we could treat the blank after the FQDN as a "missing comma", and there would be some merit to that argument. You could also argue that a "<>" quoted string would trump the occurrence of the @ earlier in the token list. However, the RFC822 grammar is designed to be parsed character by character, so that would not be a typical way for an RFC822 parser to try to do postel-style error recovery.

So, I don't think there is a bug here, but I'd be curious what other email address parsing libraries do, and that could influence whether extensions to the "make a guess when the string doesn't conform to the RFC" code would be acceptable.

bitdancer commented on Nov 6, 2018

bitdancer

on Nov 6, 2018

Member

More actions

The formatting of that doctest paragraph got messed up. Let me try again:

    >>> m = message_from_string("From: John Doe [email protected] <[email protected]>\n\n", policy=default)
    >>> m['From'].addresses
    (Address(display_name='', username='John Doe jdoe', domain='example.com'),)

tirkarthi commented on Nov 6, 2018

tirkarthi

on Nov 6, 2018

Member

More actions

Is this a case of realname having @ inside an unquoted string? As I can see from the RFC the acceptable characters of an atom other than alphabets and digits that comprises a phrase are ['!', '#', '$', '%', '&', "'", '*', '+', '-', '/', '=', '?', '^', '_', '`', '{', '|', '}', '~'] . So just curious if it's a case of @ inside unquoted string as name?

>>> for char in accepted:
...     print(parseaddr(f'John Doe jdoe{char}example.com <[email protected]>'))
...
('John Doe jdoe!example.com', '[email protected]')
('John Doe jdoe#example.com', '[email protected]')
('John Doe jdoe$example.com', '[email protected]')
('John Doe jdoe%example.com', '[email protected]')
('John Doe jdoe&example.com', '[email protected]')
("John Doe jdoe'example.com", '[email protected]')
('John Doe jdoe*example.com', '[email protected]')
('John Doe jdoe+example.com', '[email protected]')
('John Doe jdoe-example.com', '[email protected]')
('John Doe jdoe/example.com', '[email protected]')
('John Doe jdoe=example.com', '[email protected]')
('John Doe jdoe?example.com', '[email protected]')
('John Doe jdoe^example.com', '[email protected]')
('John Doe jdoe_example.com', '[email protected]')
('John Doe jdoe`example.com', '[email protected]')
('John Doe jdoe{example.com', '[email protected]')
('John Doe jdoe|example.com', '[email protected]')
('John Doe jdoe}example.com', '[email protected]')
('John Doe jdoe~example.com', '[email protected]')

>>> parseaddr('"John Doe [email protected]" <[email protected]>')
('John Doe [email protected]', '[email protected]')

>>> parseaddr('John Doe [email protected] <[email protected]>')
('', 'John Doe [email protected]')

msapiro commented on Nov 6, 2018
msapiromannequin
on Nov 6, 2018
Mannequin
More actions
I agree that my example with an @ in the 'display name', although actually seen in the wild, is non-compliant, and that the behavior of parseaddr() in this case is not a bug.

Sorry for the noise.
tirkarthi commented on Nov 6, 2018
tirkarthi
on Nov 6, 2018
Member
More actions
Ah sorry, I was typing so long and had an idle session that I didn't realize @r.david.murray added a comment with the explanation. Just to add I tried using Perl module (https://metacpan.org/release/Email-Address) that uses regex for parsing that returns me two addresses and the regex is also not much comprehensible.

use v5.14;
use Email::Address;

my $line = 'John Doe [email protected] <[email protected]>';
my @addresses = Email::Address->parse($line);
say $addresses[0];
say $addresses[1];

say "Angle address regex";
say $Email::Address::angle_addr;

[email protected]
[email protected]
Angle address regex
(?^:(?^:(?^:\s*$(?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+)<(?^:(?^:(?^:(?^:(?^:\s$(?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+)(?^:[^\x00-\x1F\x7F()<>\[\]:;@\\,."\s]+(?:\.[^\x00-\x1F\x7F()<>\[\]:;@\\,."\s]+))(?^:(?^:\s*$(?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+))|(?^:(?^:(?^:\s$(?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+)"(?^:(?^:[^\\"])|(?^:\$?^:[^\x0A\x0D])))"(?^:(?^:\s*\((?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+)))\@(?^:(?^:(?^:(?^:\s$(?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+)(?^:[^\x00-\x1F\x7F()<>\[\]:;@\\,."\s]+(?:\.[^\x00-\x1F\x7F()<>\[\]:;@\\,."\s]+))(?^:(?^:\s*$(?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+))|(?^:(?^:(?^:\s$(?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+)$?:\s*(?^:(?^:[^\[\]\\])|(?^:\\(?^:[^\x0A\x0D]))))\s\)))>(?^:(?^:\s*\((?:\s*(?^:(?^:(?>[^()\\]+))|(?^:\\(?^:[^\x0A\x0D]))|))\s$\s*)|\s+)*)

Thanks
38 remaining items
maxking commented on Aug 19, 2019
maxking
on Aug 19, 2019
Contributor
More actions
2.7 needs a separate PR since the code is very different and my familiarity with 2.7 version of email package is very limited.

I am going to work on a separate patch later this week for 2.7.
ambv commented on Aug 23, 2019
ambv
on Aug 23, 2019
Contributor
More actions
Downgraded the severity since 3.6 - 3.9 are merged.
ambv
removed
deferred-blocker
on Aug 23, 2019
larryhastings commented on Sep 7, 2019
larryhastings
on Sep 7, 2019
Contributor
More actions
New changeset 063eba2 by larryhastings (Abhilash Raj) in branch '3.5':
[3.5] bpo-34155: Dont parse domains containing @ (GH-13079) (bpo-15317)
063eba2
larryhastings commented on Sep 7, 2019
larryhastings
on Sep 7, 2019
Contributor
More actions
All PRs merged. Thanks, everybody!
larryhastings
closed this as completed on Sep 7, 2019
ret2libc commented on Sep 9, 2019
ret2libcmannequin
on Sep 9, 2019
Mannequin
More actions
CVE-2019-16056 has been assigned to this issue.
See https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-16056 .
vstinner commented on Sep 9, 2019
vstinner
on Sep 9, 2019
Member
More actions
I reopen the issue since Python 2.7 is still vulnerable.
vstinner
reopened this on Sep 9, 2019
vstinner
changed the title ~~[-]email.utils.parseaddr mistakenly parse an email[/-]~~ [+][CVE-2019-16056] email.utils.parseaddr mistakenly parse an email[/+] on Sep 9, 2019
rcsanchez97 commented on Sep 12, 2019
rcsanchez97mannequin
on Sep 12, 2019
Mannequin
More actions
I am working on Debian LTS support. I have submitted a PR that contains the necessary adjustments to implement the fix in 2.7.
miss-islington commented on Sep 14, 2019
miss-islington
on Sep 14, 2019
Contributor
More actions
New changeset 4cbcd2f by Miss Islington (bot) (Roberto C. Sánchez) in branch '2.7':
[2.7] bpo-34155: Dont parse domains containing @ (GH-13079) (GH-16006)
4cbcd2f
maxking commented on Sep 14, 2019
maxking
on Sep 14, 2019
Contributor
More actions
Merged in 2.7, closing this one finally!

Thanks to everyone who helped with this :)
maxking
closed this as completed on Sep 14, 2019
ezio-melotti
transferred this issue from on Apr 10, 2022

Metadata

Assignees

No one assigned

Labels

3.7 (EOL)end of lifeend of life3.8 (EOL)end of lifeend of life3.9 (EOL)end of lifeend of lifetopic-emailtype-securityA security issueA security issue

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

[CVE-2019-16056] email.utils.parseaddr mistakenly parse an email #78336

Description

Activity

cnicodeme commented on Jul 19, 2018

bitdancer commented on Jul 19, 2018

bitdancer commented on Jul 19, 2018

bitdancer commented on Jul 19, 2018

jwilk commented on Jul 19, 2018

msapiro commented on Nov 6, 2018

bitdancer commented on Nov 6, 2018

bitdancer commented on Nov 6, 2018

tirkarthi commented on Nov 6, 2018

msapiro commented on Nov 6, 2018

tirkarthi commented on Nov 6, 2018

38 remaining items

maxking commented on Aug 19, 2019

ambv commented on Aug 23, 2019

larryhastings commented on Sep 7, 2019

larryhastings commented on Sep 7, 2019

ret2libc commented on Sep 9, 2019

vstinner commented on Sep 9, 2019

rcsanchez97 commented on Sep 12, 2019

miss-islington commented on Sep 14, 2019

maxking commented on Sep 14, 2019

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions