Skip to content
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

Jump to bottom

[security] email module incorrect handling of CR and LF newline characters in Address objects. #83254

Closed
jap mannequin opened this issue Dec 17, 2019 · 14 comments
Closed

[security] email module incorrect handling of CR and LF newline characters in Address objects. #83254

jap mannequin opened this issue Dec 17, 2019 · 14 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes topic-email type-security A security issue

Comments

@jap
Copy link
Mannequin

jap mannequin commented Dec 17, 2019

BPO 39073
Nosy @warsaw, @vstinner, @larryhastings, @ned-deily, @bitdancer, @csabella, @miss-islington, @epicfaace, @jap
PRs
  • bpo-39073: validate Address parts to disallow CRLF #19007
  • [3.8] bpo-39073: validate Address parts to disallow CRLF (GH-19007) #19222
  • [3.7] bpo-39073: validate Address parts to disallow CRLF (GH-19007) #19223
  • [3.6] bpo-39073: validate Address parts to disallow CRLF (GH-19007) #19224
  • [3.5] bpo-39073: validate Address parts to disallow CRLF (#19007) #20450
    		•

    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 2020-07-19.09:42:15.677>
created_at = <Date 2019-12-17.12:46:43.254>
labels = ['type-security', '3.8', 'expert-email', '3.10', '3.7', '3.9']
title = '[security] email module incorrect handling of CR and LF newline characters in Address objects.'
updated_at = <Date 2020-07-19.09:42:15.676>
user = 'https://github.com/jap'

    bugs.python.org fields:

    activity = <Date 2020-07-19.09:42:15.676>
actor = 'ned.deily'
assignee = 'none'
closed = True
closed_date = <Date 2020-07-19.09:42:15.677>
closer = 'ned.deily'
components = ['email']
creation = <Date 2019-12-17.12:46:43.254>
creator = 'jap'
dependencies = []
files = []
hgrepos = []
issue_num = 39073
keywords = ['patch']
message_count = 14.0
messages = ['358544', '358545', '358572', '364273', '365287', '365288', '369659', '370076', '370077', '370080', '370081', '370151', '371386', '373948']
nosy_count = 9.0
nosy_names = ['barry', 'vstinner', 'larry', 'ned.deily', 'r.david.murray', 'cheryl.sabella', 'miss-islington', 'epicfaace', 'jap']
pr_nums = ['19007', '19222', '19223', '19224', '20450']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue39073'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']
    @jap
    Copy link
    Mannequin Author

    jap mannequin commented Dec 17, 2019

    big-bob:t spaans$ cat fak.py
    import sys

    from email.message import EmailMessage
from email.policy import SMTP
from email.headerregistry import Address

msg = EmailMessage(policy=SMTP)

a = Address(display_name='Extra Extra Read All About It This Line Does Not Fit In 80 Characters So Should Be Wrapped <dev@local>\r\nX:', addr_spec='evil@local')
msg['To'] = a
print(sys.version)
print(msg.as_string())
big-bob:t spaans$ python3.5 fak.py
3.5.2 (default, Jul 16 2019, 13:40:43) 
[GCC 4.2.1 Compatible Apple LLVM 10.0.1 (clang-1001.0.46.4)]
To: "Extra Extra Read All About It This Line Does Not Fit In 80 Characters So Should Be Wrapped <dev@local>
X:" <evil@local>

    big-bob:t spaans$ python3.8 fak.py
    3.8.0 (default, Dec 17 2019, 13:32:18)
    [Clang 11.0.0 (clang-1100.0.33.16)]
    To: Extra Extra Read All About It This Line Does Not Fit In 80 Characters So
    Should Be Wrapped <dev@local>
    X: <evil@local>

    @jap jap mannequin added 3.8 only security fixes topic-email type-security A security issue labels Dec 17, 2019
    @jap
    Copy link
    Mannequin Author

    jap mannequin commented Dec 17, 2019

    As can be seen above, 3.5 wraps the realname in a double quote, but 3.8 fails to do so. Note that 3.5 also does not add a whitespace in front of the line starting with "X:", so it is also not merged with the previous line when parsing.

    I guess we'll have to disallow \r and \n in displaynames for now.

    @bitdancer
    Copy link
    Member

    Hmm. Yes, \r\n should be disallowed in the arguments to Address. I thought it already was, so that's a bug. That bug produces the other apparent bug as well: because the X: was treated as a separate line, the previous header did not need double quotes so they are no longer added.

    So there's no 3.8 specific bug here, but there is a bug.

    @bitdancer bitdancer changed the title email regression in 3.8: folding email incorrect handling of crlf in Address objects. Dec 17, 2019
    @bitdancer bitdancer changed the title email regression in 3.8: folding email incorrect handling of crlf in Address objects. Dec 17, 2019
    @bitdancer
    Copy link
    Member

    Thanks for the PR. I've made some review comments.

    @bitdancer
    Copy link
    Member

    New changeset 614f172 by Ashwin Ramaswami in branch 'master':
    bpo-39073: validate Address parts to disallow CRLF (bpo-19007)
    614f172

    @bitdancer
    Copy link
    Member

    Thanks!

    @csabella
    Copy link
    Contributor

    There are 3 open PRs for the backport of this to 3.6, 3.7, and 3.8. It looks like they just need to be approved and miss-islington will take care of the rest.

    @miss-islington
    Copy link
    Contributor

    New changeset 75635c6 by Miss Islington (bot) in branch '3.8':
    bpo-39073: validate Address parts to disallow CRLF (GH-19007)
    75635c6

    @miss-islington
    Copy link
    Contributor

    New changeset a93bf82 by Miss Islington (bot) in branch '3.7':
    bpo-39073: validate Address parts to disallow CRLF (GH-19007)
    a93bf82

    @vstinner
    Copy link
    Member

    I created PR 20450: backport to 3.5, since it's a security fix.

    @vstinner vstinner added 3.7 (EOL) end of life 3.9 only security fixes 3.10 only security fixes labels May 27, 2020
    @vstinner
    Copy link
    Member

    FYI I created https://python-security.readthedocs.io/vuln/email-address-header-injection.html to track fixes of this vulnerability.

    @vstinner vstinner changed the title email incorrect handling of crlf in Address objects. [security] email module incorrect handling of CR and LF newline characters in Address objects. May 27, 2020
    @vstinner vstinner changed the title email incorrect handling of crlf in Address objects. [security] email module incorrect handling of CR and LF newline characters in Address objects. May 27, 2020
    @ned-deily
    Copy link
    Member

    New changeset 7df32f8 by Miss Islington (bot) in branch '3.6':
    bpo-39073: validate Address parts to disallow CRLF (GH-19007) (bpo-19224)
    7df32f8

    @larryhastings
    Copy link
    Contributor

    New changeset f91a0b6 by Victor Stinner in branch '3.5':
    bpo-39073: validate Address parts to disallow CRLF (bpo-19007) (bpo-20450)
    f91a0b6

    @ned-deily
    Copy link
    Member

    Merged for release in 3.9.0a6, 3.8.4, 3.7.8, 3.6.11, and 3.5.10.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes topic-email type-security A security issue
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    6 participants
    @vstinner @bitdancer @ned-deily @larryhastings @csabella @miss-islington