Skip to content

Update attachment headers#277

Open
NeonDaniel wants to merge 1 commit intokootenpv:masterfrom
NeonDaniel:FIX_AttachmentHeaderCompat
Open

Update attachment headers#277
NeonDaniel wants to merge 1 commit intokootenpv:masterfrom
NeonDaniel:FIX_AttachmentHeaderCompat

Conversation

@NeonDaniel
Copy link
Copy Markdown

Refactor attachment MIME object to match other (working) providers per ProtonMail support
Closes #228

I validated this change by sending an email to my gmail address (daniel@neongecko.com) and to a newly-created ProtonMail account. Both emails contained attachments as ecpected. A sample new header is included below.

Content-Type: text/plain; name="skills_log.txt"
MIME-Version: 1.0
Content-Disposition: attachment; filename="skills_log.txt"; name="skills_log.txt"
Content-Transfer-Encoding: base64

@kootenpv
Copy link
Copy Markdown
Owner

Thank you! Could you provide an example in which it does not work, and for which it will now work? I'd love to support it, but it is hard for me to verify at the moment that it will improve the situation.

@NeonDaniel
Copy link
Copy Markdown
Author

From the linked issue, protonmail recipient accounts would discard attachments sent with yagmail, but with this change they now show up as expected. I tested this by creating a new Protonmail email account and checking attachments in their email web app.

The old header looked like:

Content-Type: text/plain; name*=utf-8''voice_log.txt
MIME-Version: 1.0
Content-Transfer-Encoding: base64

and the new header:

Content-Type: text/plain; name="skills_log.txt"
MIME-Version: 1.0
Content-Disposition: attachment; filename="skills_log.txt"; name="skills_log.txt"
Content-Transfer-Encoding: base64

A sample call might look like:

        with yagmail.SMTP("developers@neon.ai", password, "smtp.gmail.com", '465') as yag:
            yag.send(to="djmcknight358@protonmail.com", subject="Test Attachments, contents="See attached files.",
                     attachments=["/tmp/test_attach.log"])

@NeonDaniel
Copy link
Copy Markdown
Author

@kootenpv Anything else I can contribute here to help get this merged?

@mchubby
Copy link
Copy Markdown

mchubby commented Dec 11, 2024

Hello,
The ASVS Working Group at OWASP discussed ways to properly encode the Content-Disposition header. It is important to avoid the filename being misused to manipulate other headers. I kinda trust them more to navigate the flow of numerous RFCs around this subject.
OWASP/ASVS#1390

You may also refer to https://blog.nodemailer.com/2017/01/27/the-mess-that-is-attachment-filenames/ who claim you can also set Content-Type... name as a fallback value for legacy MUAs.

EDIT: Sorry, RFC 6266 is standard for HTTP. The relevant one for MTA is still RFC 2231 AFAICT. See CVE-2024-39929 for an example on how the header could be abused to send attachments with malicious names.

@joaomcarlos
Copy link
Copy Markdown

In a few days this pull request will be 1yo.

Happy birthday! (in advance)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sending attachment fails

4 participants