Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-121650 : detect newlines in headers#121812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-121650 : detect newlines in headers #121812
Uh oh!
There was an error while loading. Please reload this page.
Conversation
basbloemsaat commented Jul 15, 2024 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
ZeroIntensity left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to confirm that this PR fixes #121650. The original reproducer still contains the embedded newline:
fromemailimportmessage_from_stringfromemail.policyimportdefaultemail_in="""\To: [email protected]From: External Sender <[email protected]>Subject: Here's an =?UTF-8?Q?embedded_newline=0A?=Content-Type: text/html; charset=UTF-8Content-Transfer-Encoding: quoted-printableMIME-Version: 1.0<html><head><title>An embeded newline</title></head><body> <p>I sent you an embedded newline in the subject. How do you like that?!</p></body></html>"""msg=message_from_string(email_in, policy=default) msg=message_from_string(email_in, policy=default) forheader, valueinmsg.items(): delmsg[header] msg[header] =valueemail_out=str(msg) print(email_out)Uh oh!
There was an error while loading. Please reload this page.
Misc/NEWS.d/next/Library/2024-07-15-21-43-38.gh-issue-121650.A0FkCh.rst Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Peter Bierma <[email protected]>
…0FkCh.rst Co-authored-by: Peter Bierma <[email protected]>
basbloemsaat commented Jul 16, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I missed headers that were parsed from a message, as in original issue. I updated the fix and the tests. |
ZeroIntensity left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that this fixes #121650. I'm pretty sure this is a security fix (as you could previously inject email headers using this method), so this should need a backport all the way to 3.8
encukou commented Jul 16, 2024
@warsaw, @bitdancer, @maxking: as the email experts, do you have any comments? |
encukou left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix looks good to me! Thank you for digging into it!
encukou commented Jul 19, 2024
I take back the review. There's more to this, unfortunately :( Here's another reproducer: fromemailimportmessage_from_stringfromemail.policyimportdefaultemail_in="""\To: [email protected]From: External Sender <[email protected]> =?UTF-8?Q?embedded_newline=0A?=Smuggled-Data: BadSubject: foo <bar> Here's anContent-Type: text/html; charset=UTF-8Content-Transfer-Encoding: quoted-printableMIME-Version: 1.0<html><head><title>An embeded newline</title></head><body> <p>I sent you an embedded newline in the subject. How do you like that?!</p></body></html>"""msg=message_from_string(email_in, policy=default) print(msg) forheader, valueinmsg.items(): delmsg[header] msg[header] =valueemail_out=str(msg) print(email_out) |
basbloemsaat commented Jul 19, 2024
I'll look into this... |
…com:basbloemsaat/cpython into fix-issue-121650-detect-newlines-in-headers
basbloemsaat commented Jul 19, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@encukou I tried all header types. This eliminates all newlines. Two notes:
|
encukou commented Jul 24, 2024
After reading up on the email module, I propose to fix the issue in a different part of the code: see #122233. |
encukou commented Aug 1, 2024
Closing in favour of #122233. |
@encukou as promised, the fix for this issue.
This PR fixes both issues addressed in the issue, both the newlines at the end as well as the encoded newlines.
As this has security implication, it may need to be backported as well.