gh-121284: Fix email address header folding with parsed encoded-word by medmunds · Pull Request #122754 · python/cpython · GitHub

[This fixes a #security-issue. PSRT instructed me to handle the fix publicly.]

Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded.

The issue is very similar to PR #122753 (and has the same security implications), but this PR involves refolding an encoded-word; the other PR involves refolding a quoted-string. The fixes required are different.

Issue: email: invalid RFC 2047 address header after refolding with email.policy.default #121284

…-word Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded.

Requesting a review from @serhiy-storchaka, who recently reviewed a similar security fix around encoded words in email headers (#122233).

And pinging @bitdancer, who probably knows the most about this section of the code.

(Also hoping both of you might be able to review PR #122753, which tries to fix a related security issue first reported 5 years ago.)

(A nicer fix would be to decide separately whether each refolded segment needs rfc2047 encoding, quoted-string handling, or no special treatment. But that would require giving _refold_parse_tree() info about whether it's working in a structured or unstructured header, which seems too involved for a security patch.)

Btw, although this may seem like it's too obscure to matter much, it's actually pretty easy to stumble into vulnerable code. E.g., calling email.utils.formataddr() (correctly!) with user-supplied content can generate exactly the sort of (valid) encoded-word that gets mishandled by email.policy.default.

By my reading of the code, this is correct.
I'd appreciate an additional review from an email expert, but, if there are no objections I plat to merge this next week.

Please give me another day to look at this before you merge. I think it's fine, but there are a couple things I want to check.

OK, there is actually a pretty straightforward solution to this problem, and the functionality is better:

diff --git a/Lib/email/_header_value_parser.py b/Lib/email/_header_value_parser.py
index c0e856306d3..9a51b943733 100644
--- a/Lib/email/_header_value_parser.py
+++ b/Lib/email/_header_value_parser.py
@@ -1053,7 +1053,7 @@ def get_fws(value):
     fws = WhiteSpaceTerminal(value[:len(value)-len(newvalue)], 'fws')
     return fws, newvalue
 
-def get_encoded_word(value):
+def get_encoded_word(value, terminal_type='vtext'):
     """ encoded-word = "=?" charset "?" encoding "?" encoded-text "?="
 
     """
@@ -1092,7 +1092,7 @@ def get_encoded_word(value):
             ew.append(token)
             continue
         chars, *remainder = _wsp_splitter(text, 1)
-        vtext = ValueTerminal(chars, 'vtext')
+        vtext = ValueTerminal(chars, terminal_type)
         _validate_xtext(vtext)
         ew.append(vtext)
         text = ''.join(remainder)
@@ -1134,7 +1134,7 @@ def get_unstructured(value):
         valid_ew = True
         if value.startswith('=?'):
             try:
-                token, value = get_encoded_word(value)
+                token, value = get_encoded_word(value, 'utext')
             except _InvalidEwError:
                 valid_ew = False
             except errors.HeaderParseError:
@@ -1163,7 +1163,7 @@ def get_unstructured(value):
         # the parser to go in an infinite loop.
         if valid_ew and rfc2047_matcher.search(tok):
             tok, *remainder = value.partition('=?')
-        vtext = ValueTerminal(tok, 'vtext')
+        vtext = ValueTerminal(tok, 'utext')
         _validate_xtext(vtext)
         unstructured.append(vtext)
         value = ''.join(remainder)
@@ -2813,7 +2813,7 @@ def _refold_parse_tree(parse_tree, *, policy):
             continue
         tstr = str(part)
         if not want_encoding:
-            if part.token_type == 'ptext':
+            if part.token_type in ('ptext', 'vtext'):
                 # Encode if tstr contains special characters.
                 want_encoding = not SPECIALSNL.isdisjoint(tstr)
             else:

At this point I no longer remember what 'vtext' was supposed to stand for, but 'utext' is obviously 'unstructured text token' ;) The documentation of these bits of the code could use some improvement (not to mention the code itself!), but this fixes the problem pretty much in the way the original code was intended to work, if we imagine that the failure to check whether or not we were dealing with structured text was a bug as opposed to me forgetting about the distinction ;)

… encoded-word [Better fix from @bitdancer.] Co-authored-by: R David Murray <rdmurray@bitdance.com>

Nice! That feels much better, and allows unencoding encoded-words in refolding when it's safe. I've updated the PR with your change.

LGTM

…-word (pythonGH-122754) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] --------- (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

GH-131403 is a backport of this pull request to the 3.13 branch.

…-word (pythonGH-122754) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] --------- (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

GH-131404 is a backport of this pull request to the 3.12 branch.

…-word (pythonGH-122754) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] --------- (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

Sorry, @medmunds and @encukou, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 295b53df2aa18deb625a7da41f7e4babfe6ef34b 3.10

GH-131405 is a backport of this pull request to the 3.11 branch.

Sorry, @medmunds and @encukou, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 295b53df2aa18deb625a7da41f7e4babfe6ef34b 3.9

…encoded-word (pythonGH-122754) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] --------- (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

GH-131411 is a backport of this pull request to the 3.10 branch.

…ncoded-word (pythonGH-122754) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] --------- (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

GH-131412 is a backport of this pull request to the 3.9 branch.

…d-word (GH-122754) (#131403) gh-121284: Fix email address header folding with parsed encoded-word (GH-122754) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] --------- (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

…d-word (GH-122754) (#131404) gh-121284: Fix email address header folding with parsed encoded-word (GH-122754) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] --------- (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

…-word (pythonGH-122754) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] --------- Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

…d-word (GH-122754) (GH-131405) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com> Co-authored-by: Petr Viktorin <encukou@gmail.com>

…d-word (GH-122754) (GH-131411) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com>

…-word (GH-122754) (GH-131412) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com>

…ncoded-word (pythonGH-122754) (pythonGH-131412) Email generators using email.policy.default may convert an RFC 2047 encoded-word to unencoded form during header refolding. In a structured header, this could allow 'specials' chars outside a quoted-string, leading to invalid address headers and enabling spoofing. This change ensures a parsed encoded-word that contains specials is kept as an encoded-word while the header is refolded. [Better fix from @bitdancer.] (cherry picked from commit 295b53d) Co-authored-by: Mike Edmunds <medmunds@gmail.com> Co-authored-by: R David Murray <rdmurray@bitdance.com>

And a small bugfux: BUGFIX: pythongh-121284/pythonGH-122754 introduced the ability to specify the terminal type in get_encoded_word, but only the get_unstructured calls were updated to use it. It remained the case that a quoted string ended up with a mixture of 'ptext' and 'vtext' tokens. The folder teats these the same, so that did not cause any problems. The parser now correctly makes all terminal tokens in quoted string content be of type 'ptext'.

medmunds requested a review from a team as a code owner August 6, 2024 19:50

bedevere-app Bot mentioned this pull request Aug 6, 2024

email: invalid RFC 2047 address header after refolding with email.policy.default #121284

Closed

bedevere-app Bot added the awaiting review label Aug 6, 2024

sethmlarson added type-security A security issue topic-email labels Aug 7, 2024

picnixz requested a review from serhiy-storchaka September 9, 2024 19:01

Merge branch 'main' into fix-issue-121284

9c05613

medmunds mentioned this pull request Oct 29, 2024

gh-80222: Fix email address header folding with long quoted-string #122753

Merged

medmunds added 2 commits January 18, 2025 14:06

Merge branch 'main' into fix-issue-121284

a8d23d2

Be more specific in news

45159cd

encukou approved these changes Feb 27, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Feb 27, 2025

medmunds and others added 2 commits March 5, 2025 10:32

Merge branch 'main' into fix-issue-121284

e63ec20

squash! pythongh-121284 : Fix email address header folding with parsed…

c249511

… encoded-word [Better fix from @bitdancer.] Co-authored-by: R David Murray <rdmurray@bitdance.com>

bitdancer approved these changes Mar 6, 2025

View reviewed changes

Merge branch 'main' into fix-issue-121284

1da1d57

bitdancer added needs backport to 3.9 needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Mar 17, 2025

Merge branch 'main' into fix-issue-121284

ede0ce8

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Mar 18, 2025

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Mar 18, 2025

miss-islington-app Bot assigned encukou Mar 18, 2025

bedevere-app Bot removed the needs backport to 3.11 only security fixes label Mar 18, 2025

bedevere-app Bot removed the needs backport to 3.10 only security fixes label Mar 18, 2025

bedevere-app Bot removed the needs backport to 3.9 label Mar 18, 2025

medmunds mentioned this pull request May 6, 2025

EmailMessage objects break when folding malformed header #132105

Open

gh-121284: Fix email address header folding with parsed encoded-word by medmunds · Pull Request #122754 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-121284: Fix email address header folding with parsed encoded-word - #122754

gh-121284: Fix email address header folding with parsed encoded-word#122754encukou merged 8 commits intopython:mainpython/cpython:mainfrom medmunds:fix-issue-121284medmunds/cpython:fix-issue-121284Copy head branch name to clipboard

Conversation

medmunds commented Aug 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

medmunds commented Sep 9, 2024

Uh oh!

medmunds commented Jan 18, 2025

Uh oh!

medmunds commented Jan 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

encukou left a comment

Choose a reason for hiding this comment

Uh oh!

bitdancer commented Mar 4, 2025

Uh oh!

bitdancer commented Mar 5, 2025

Uh oh!

medmunds commented Mar 5, 2025

Uh oh!

bitdancer left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Mar 18, 2025

Uh oh!

bedevere-app Bot commented Mar 18, 2025

Uh oh!

miss-islington-app Bot commented Mar 18, 2025

Uh oh!

bedevere-app Bot commented Mar 18, 2025

Uh oh!

miss-islington-app Bot commented Mar 18, 2025

Uh oh!

bedevere-app Bot commented Mar 18, 2025

Uh oh!

bedevere-app Bot commented Mar 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gh-121284: Fix email address header folding with parsed encoded-word#122754
encukou merged 8 commits into
python:mainfrom
medmunds:fix-issue-121284

medmunds commented Aug 6, 2024 •
edited
Loading

medmunds commented Jan 19, 2025 •
edited
Loading