WordPress.org

Make WordPress Core

Opened 8 weeks ago

Last modified 3 days ago

#53393 new defect (bug)

Latest version of PHPMailer no longer attaches files to emails

Reported by: threeatetwo Owned by:
Milestone: 5.8.1 Priority: normal
Severity: normal Version: 5.7.2
Component: Mail Keywords: needs-testing
Focuses: Cc:

Description

I've been trying to track down why my s3 bucket files are no longer being attached to automated emails being sent out by WordPress (using the Gravity Forms Entry Automation plugin).

I've been able to identify the latest version of PHPMailer being the reason why the attachments no longer get added. From the author:


This is due to a bug fix that also closed a major security hole. PHPMailer deliberately avoids being a client for HTTP or other protocols for file attachments because it is far too risky. The solution is to take on responsibility for fetching remote files yourself. Instead of :

$mail->addAttachment('s3://file.txt', 'file.txt');

Do this:

$mail->addStringAttachment(file_get_contents('s3://file.txt'), 'file.txt');

https://github.com/PHPMailer/PHPMailer/issues/2355#issuecomment-858373284


I reached out to the Gravity Forms authors as well as the Entry Automation authors and they both have said their plugins just generate raw notification objects and then use wp_mail() to pass the generated mail object off to the rest of my server to actually handle the sending.

Is there a way to get this working again without having to roll my WP version back? Or do y'all know how others are handling this issue?

Change History (6)

#1 @SergeyBiryukov
7 weeks ago

  • Component changed from General to External Libraries

#2 @threeatetwo
7 weeks ago

Would it be possible in wp_mail to add a filter to handle attachment URLs and fetch the contents? My temporary fix right now is I copied the wp_mail function into a plugin and modified it like so:

    if ( ! empty( $attachments ) ) {
      foreach ( $attachments as $attachment ) {
        try {
          if ( substr($attachment, 0, 20 ) === "s3://mybucketnamehere" ) {
            $strArray = explode('/',$attachment);
            $filename = end($strArray);
            $phpmailer->addStringAttachment( file_get_contents($attachment ), $filename);
          } else {
            $phpmailer->addAttachment( $attachment );
          }
        } catch ( PHPMailer\PHPMailer\Exception $e ) {
          continue;
        }
      }
    }

#3 @desrosj
6 weeks ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 5.8.1

Sorry that someone was not able to investigate this one before the deadline to include in 5.8. I'm moving this to 5.8.1 to get it some attention.

#4 @desrosj
6 weeks ago

  • Component changed from External Libraries to Mail

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


13 days ago

#6 @peterwilsoncc
3 days ago

Introduced in [50799] and [50800] for #53114. If I recall correctly, some of these changes were ported in to earlier versions of WP too.

Given the relative popularity of using streams in WP (s3 and other storage bucket plugins are quite popular), I'd like to support streams for attaching files to email.

I'm hesitant about something as simple as:

<?php
if ( is_wp_stream( /* file */ ) ) {
  $phpmailer->addStringAttachment( file_get_contents( /* file */ ) );
}

Such code include HTTP, FTP and several similar protocols by default. Essentially reintroducing to WP the problem the PHPMailer library was trying to avoid.

It's WP's prerogative to loosen the restrictions on streams to account for acceptable and safe use within the user base, care will need to be taken not to loosen the restrictions too far.

Note: See TracTickets for help on using tickets.