Make WordPress Core

Opened 14 years ago

Closed 12 years ago

Last modified 7 years ago

#14888 closed defect (bug) (wontfix)

PHPMailer class uses wrong/no sender for mail envelope

Reported by: gkusardi's profile gkusardi Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: External Libraries Keywords: has-patch
Focuses: Cc:

Description

As an result following SMTP envelope and "Return-Path" email address can occur:

apache@localhost.localdomain

This will lead some mail servers to reject mails because of the not valid fqdn "localhost.localdomain".

Correctly the defined FROM address should be used, usually:

'wordpress@'.$_SERVER['SERVER_NAME']

In PHPMailer::CreateHeader() it is checked for

if($this->Sender == '')

The following will set the "Return-Path" in the mail headers, but NOT the envelope email address used in the SMTP handshake:

$result .= $this->HeaderLine('Return-Path', trim($this->From));

I fixed it by adding the following line after the mentioned one, setting the "Return-Path" in the mail header:
$this->Sender = $this->From;

So my condition looks like:

    if($this->Sender == '') {
      $result .= $this->HeaderLine('Return-Path', trim($this->From));
      $this->Sender = $this->From;
    } else {
      $result .= $this->HeaderLine('Return-Path', trim($this->Sender));
    }

Attachments (3)

class-phpmailer.php.diff (502 bytes) - added by gkusardi 14 years ago.
wp-phpmailer.diff (851 bytes) - added by LynxChaus 14 years ago.
14888.patch (586 bytes) - added by SergeyBiryukov 13 years ago.

Download all attachments as: .zip

Change History (28)

#1 @hakre
14 years ago

Thanks for reporting. Can you provide a patch file? This would help to better deal with the issue.


X-Ref: #5007

#2 @gkusardi
14 years ago

I've added a diff patch file.

#3 @hakre
14 years ago

Thanks for providing a patch. I like the idea. +1

#4 @LynxChaus
14 years ago

RFC2821 allow adding Return-Path only when LDA deliver message, so - remove this crap completely.

#5 @westi
14 years ago

  • Cc westi added
  • Keywords reporter-feedback added
  • Milestone changed from Awaiting Review to Future Release

How do I reproduce this issue?

#6 @tigertech
13 years ago

I want to second the comment from LynxChaus: This proposed change is a bad idea that will cause some systems to discard the mail.

RFC 2821 section 4.4 says: "A message-originating SMTP system SHOULD NOT send a message that already contains a Return-path header". If WordPress submits messages that include a "Return-Path" header via a SMTP system that enforces this rule, the message will not be delivered.

Please don't do this. It will break things.

#7 follow-up: @cnorris23
13 years ago

  • Keywords close added

This seems to have been addressed at least in PHPMailer 5.2 around line 766, code below:

$smtp_from = ($this->Sender == '') ? $this->From : $this->Sender;

Close?

#8 @tigertech
13 years ago

My 2 cents: I agree that the PHPMailer solution is the correct one, and that this "Return-Path" idea should be closed.

#9 in reply to: ↑ 7 @SergeyBiryukov
13 years ago

  • Keywords close removed

Replying to cnorris23:

This seems to have been addressed at least in PHPMailer 5.2 around line 766, code below:

$smtp_from = ($this->Sender == '') ? $this->From : $this->Sender;

That line exists since PHPMailer was introduced in #3862:
http://core.trac.wordpress.org/browser/tags/2.2/wp-includes/class-phpmailer.php#L467
It doesn't seem to address the issue reported here though.

If we should remove "Return-Path", then wp-phpmailer.diff probably makes sense, but I don't get what the first change (removing $smtp_from) is there for. Looks like only the second one is needed.

#10 @ipublicis
13 years ago

  • Keywords has-patch added

#11 follow-up: @basos
13 years ago

  • Version changed from 3.0 to 3.2.1

Hello, SergeyBiryukov patch is correct (partially). Should remove reply-to headers explicitly being set AND should remove sender to be auto-filled with from address. I.E. remove all lines.

Explanations

Note that this patch addresses the issue where "wordpress generated emails do not have an appropriate envelope address (reply-to header)", which could cause servers to reject mail (when envelope is not a fqdn - misconfiguration) or classify them as SPAM (when envelope is a shared system domain, see below). Also should the default envelope be a valid address, the bounces will go there, and not in a configurable (webmaster) email.

On the other hand there is another issue (the old #5273), that will reappear if this patch is applied. This is the "wordpress generated emails -erroneously- specify the from header address as a non configurable wordpress[@]sitedomain.com email", which when combined with the envelope address fix above, will cause some recipient SMTPs to reject the mail due to callback (callout) verification failure.

The solutuion

  1. At php mailer do not assume anything about sender i.e. leave it blank by default (to instruct sending SMTP to use it's default envelope). Also apply the fix to not explicitly set return-path sender as it is against the specs.
  2. Add a new wp configuration option "System email" i.e. "An email address to be used for bounces. This should be an existing email address at the originating mail server. Leave it blank to use the server's default address.". Normally, this would be the webmaster's email (the technical guy). Then, before sending emails, set the PHPMailer's sender explicitly to this address.
  3. Use the existing wp "admin_email" as a from header to all wordpress generated emails. This will be the email that users will see as a sender (and possibly reply to). Normally the blog administrator's email (the social guy :).

Also a small history, to explain how I found thing trac report (and an example of the impact on spam filters):

In my case the envelope sender (reply-to header) was added automatically by the shared hosting provider.(By the way it is a mail server misconfiguration to set envelope sender address to a non-fqdn)
Instead of being someone[@]mydomain.com it was servername[@]companyserver.com. This caused some SPAM filtering engines on the recipient side (possibly having to do with SPF) to classify the mail as SPAM. (Meanwhile I found that the server's SMTP address is at a SPAM list, which is another story). Nevertheless, when the correct envelope sender was set, the mail was not classified as SPAM at least at yahoo and gmail.
Also the two scenarios have a differed Receive-SPF header. The latter being a "best guess" pass, whatever that could mean.

  • with envelope set to @mydomain.com

Received-SPF: Pass (recipientmailprovider.com: domain of wordpress[@]mydomain.com designates xxx.xxx.xxx.xxx as permitted sender ) client-ip: xxx.xxx.xxx.xxx

  • with no-envelope set (defaulting to sending SMTP server's envelope @companyserver.com)

Received-SPF: Pass (recipientmailprovider.com: domain of servername[@]companyserver.com designates xxx.xxx.xxx.xxx as permitted sender using best guess ) client-ip: xxx.xxx.xxx.xxx

Last edited 13 years ago by basos (previous) (diff)

#12 @SergeyBiryukov
13 years ago

  • Version changed from 3.2.1 to 3.0

Version number indicates when the bug was initially introduced/reported.

#13 in reply to: ↑ 11 ; follow-up: @tigertech
13 years ago

Replying to basos:

Instead of being someone[@]mydomain.com it was servername[@]companyserver.com. This caused some SPAM filtering engines on the recipient side (possibly having to do with SPF) to classify the mail as SPAM. (Meanwhile I found that the server's SMTP address is at a SPAM list, which is another story). Nevertheless, when the correct envelope sender was set, the mail was not classified as SPAM at least at yahoo and gmail.

Ummm... It feels like you're using the term "correct envelope sender" to simply mean "an address that didn't trigger spam filters in my particular situation".

Yes, a default e-mail address that your ISP uses was flagged as spam, but that doesn't necessarily mean WordPress should treat all such addresses as less "correct". Another WordPress user might randomly have the exact opposite problem: the ISP-supplied default address works, but a manually entered address (or a made-up address like "wordpress@domain") doesn't. There are plenty of reports of that situation, too.

I'm sure that a made-up address is the wrong approach, so that leaves the question as "should WordPress ask users to manually enter an envelope sender address"? My suspicion is that that would not help, and would probably hurt. People have no idea what envelope sender addresses their hosting company allows. In particular, many users would probably enter a freemail address that's unrelated to their WordPress site domain name, which many hosting companies would block (my company certainly doesn't allow scripts to start sending mail from random domain names, since that usually indicates a spam exploit). Even if the company does allow it, it probably won't pass an SPF check and is thus probably *more* likely to be flagged as spam than an address the hosting company set up.

No solution is perfect, but the reality is that a default outgoing address *should* work. This discussion attracts anecdotal reports to the contrary because only people who have trouble are likely to bother posting, but keep in mind that the default outgoing address is something that all competent hosting companies are aware of and do try to maintain properly. Almost every PHP script that sends mail uses the default envelope sender address, and if it doesn't work, we hear about that pretty quickly.

#14 in reply to: ↑ 13 @basos
13 years ago

Replying to tigertech:

Replying to basos:

Ummm... It feels like you're using the term "correct envelope sender" to simply mean "an address that didn't trigger spam filters in my particular situation".

Ok, you are right for this in the sense that I do not bother to comment on this only because of the hypothetical SPAM remedy. The only observable difference that relates with SPAM, is the "best guess" string at the SPF authentication when the default envelope is used.

My proposal for the new "system email address" option, is mainly for bounces (i.e. wordpress generated emais that didn't make it to the receiver). When you leave your SMTP server to decide for the default envelope your bounces will go to an address that you didn't consider.

I'm sure that a made-up address is the wrong approach, so that leaves the question as "should WordPress ask users to manually enter an envelope sender address"? My suspicion is that that would not help, and would probably hurt. People have no idea what envelope sender addresses their hosting company allows. In particular, many users would probably enter a freemail address that's unrelated to their WordPress site domain name, which many hosting companies would block (my company certainly doesn't allow scripts to start sending mail from random domain names, since that usually indicates a spam exploit).

I totally agree that custom envelop is not to be used with arbitrary emails. Rather it is to be used with legitimate, existing email addresses on the sending SMTP machine. This should be optional and targeted to advanced users that need precise control of the email sending procedure.

For example a hypothetical setup would be: on the wordpress machine (also acting as an SMTP server) create a webmaster[@]mydomain.com email. Then, define this email as the system email to be used as an envelope. This is a legit usage of explicit envelope setting and would not be blocked by your hosting company. Then, all the bounces will return to the webmaster's address.

For the reference this functionality exists at the phpbb forum webapp as a "Return e-mail address" configuration option.

PS: I have already patched 3.4.1 and could post patches if it is needed.

#15 follow-up: @Whissi
12 years ago

  • Cc Whissi added

OK, let me summarize:

  • Your domain is example.org.
  • PHP's sendmail directive isn't set.
  • PHP is running as user "vhost123".
  • The hostname of the server is "ded4321.fw2.dc7.hosting-company.invalid".

When your WordPress installation example.org/blog will now send an email (e.g. Lost Password), WordPress will set the header value "FROM: wordpress[@]example.org", but the mail envelope sender will be "vhost123[@]ded4321.fw2.dc7.hosting-company.invalid".

There is currently no way to set an envelope sender address, right?

Other application give you the option to specify the "sendmail -f" option for example, but WordPress doesn't have such a feature.

Result:
Any rules targeting wordpress[@]example.org at sender level won't work, because this is not the sender.

You are concerned (@tigertech) that the average WordPress user would set the "wrong" address, if there would be such an option? Really, we don't have to talk about SPF at this place. SPF is failed by design. Forwardings mails is a basic feature, which is broken by SPF. So you are really concerned about breaking SPF by WordPress?!

Are you arguing, that the hosting company should set PHP sendmail directive? That would be a very limiting factor: You could have more than just WordPress running. If this would be right for WordPress, it would be also right for other applications. But what should a user do, who wants that the forum application sends mail using forum@… as sender, the shop system should send mails using the orders[@]example.org sender address and the blog should use blog[@]example.org? This wouldn't be possible in your "world". I hope you see, why this argumentation must be wrong.

Every application should be able to specify the sender address.

And if you fear WordPress could be used for SPAM: It is the administrator's job to prevent that. If the hosting provider doesn't want that the user "vhost123" can send mails using any address, the provider is able to block that. Don't try to solve other people problems. And remember: The user is already able to specify a SMTP server for outgoing mails...

I would vote for basos solution (comment 11). You may add it to the configuration file, not to wp-admin interface, like you did it with other "critical" options, normal users shouldn't use.

@basos:
Could you please show us your patch you mentioned in comment 14?

#16 @Whissi
12 years ago

Just a clarification:
sendmail_from won't be used in non-Windows environments. So you have to add "-f your-desired-sender-name[@]example.org" to sendmail_path directive... but again, WordPress should offer a way to specify a sender... that's an application task.

#17 in reply to: ↑ 15 ; follow-up: @tigertech
12 years ago

Replying to Whissi:

You are concerned (@tigertech) that the average WordPress user would set the "wrong" address, if there would be such an option?

Yes, that's exactly what I'm concerned about.

The correct envelope from address in this case is "vhost123[@]ded4321.fw2.dc7.hosting-company.invalid". It's up to the hosting company to make sure that whatever address is used there is an address that works. The idea that they have done so is a reasonable expectation, since if they hadn't, lots of PHP scripts on their servers would be unusable.

If you let users enter some random thing there, they're going to enter "something@…", for example, and then it quite definitely won't work in some cases.

Really, we don't have to talk about SPF at this place. SPF is failed by design. Forwardings mails is a basic feature, which is broken by SPF. So you are really concerned about breaking SPF by WordPress?!

Yes. SPF is used on the Internet to reject mail by lots of large ISPs, including GoDaddy.

If you're going to argue that it's okay to ignore SPF in this non-forwarding case because SPF separately breaks forwarding... well, that's a non-starter as an argument, in my opinion.

You're also ignoring DKIM. Some domain names now publish records telling recipients to discard all unsigned mail claiming to be from their domain name. That's not widespread, but letting people bung arbitrary from addresses into WordPress could also break that.

More generally, you're focusing on a specific piece of technology, SPF (or DKIM, or SMTP callbacks, or whatever else this might break), but that's too narrow a focus. What people seem to be missing is that regardless of SPF, or DKIM, or anything else, it's just generally a bad idea to send mail claiming to be from (say) gmail.com if your mail server isn't gmail.com. There are all sorts of possible reasons that some recipients will think you're forging headers if you do that (including naive custom filters on the receiving end), and the mail won't be delivered. The average user isn't going to expect that problem.

Last edited 7 years ago by tigertech (previous) (diff)

#18 @bpetty
12 years ago

  • Cc bpetty added
  • Component changed from Mail to External Libraries

I'm confused, what is this ticket claiming is a bug in WordPress?

It sounds like if there are any proposed changes here, it's definitely in the PHPMailer source code, which is a 3rd party library. This means any proposed changes need to be reported (and fixed) in an upstream release of PHPMailer.

#19 in reply to: ↑ 17 ; follow-up: @Whissi
12 years ago

Replying to tigertech:

More generally, you're focusing on a specific piece of technology, SPF (or DKIM, or SMTP callbacks, or whatever else this might break), but that's too narrow a focus. What people seem to be missing is that regardless of SPF, or DKIM, or anything else, it's just generally a bad idea to send mail claiming to be from (say) gmail.com if your mail server isn't gmail.com. There are all sorts of possible reasons that some recipients will think you're forging headers if you do that (including naive custom filters on the receiving end), and the mail won't be delivered. The average user isn't going to expect that problem.

I partly agree: When you are not authorized to use a specific domain, you shouldn't use this domain. Right. But it would be another discussion, if you are authorized to send as "foo[@]gmail.com", when you "own" this address. Currently, we cannot make this decission per address. When we assume we are (and I do!) authorized, then it is ok.
Well, because we are using a domain someone else is owning, we have to deal with the email policy the owner has published (e.g. SPF, DKIM...) but that's the problem of the person, who wants to send as "foo[@]gmail.com".

Because I also have a background as administrator, I can understand your wish to prevent somebody from doing things he/she shouldn't do. But you cannot. And more important: You shouldn't! As administrator you should just care about your systems and your domains. If you want to use SPF or something else, it is you choice to do that. You can say "Nobody expected my server 1.2.3.4 is allowed to send mails as [@]example.org". That's ok. But when my server for example will get a mail from someone claiming to be you ([@]example.org), it is my decission if I will do some checks. I can see via DNS that you, the owner of example.org, has published any mail policies, but it is my decission if I will follow your policy and block the mail, because it wasn't send from 1.2.3.4 or any other check your policy requires failed.

I would fully agree with you, when there wouldn't be any legit reasons to set a sender via software. But there are reasons (as I mentioned before).

Coming back to this ticket:
I would close it as invalid, because it is the server used by gkusardi, which isn't configured properly. Because WordPress cannot determine the right email domain, it shouldn't set any.

But I still vote for a feature which would allow you to set the envelope sender via WordPress. At least via PHP constant, like we use for other expert settings.

For now, we can use PHP's .user.ini or PATH directive to set PHP's sendmail_* settings per directory. This would at least allow someone to use multiple application in one domain with different sender addresses like shop[@]example.org for the online-shop application in /shop and wordpress[@]example.org for the blog running in /blog in the same domain.

@ tigertech: If you would still vote against such a feature, could you please explain why? I mean, setting it via .user.php/PHP configuration or WordPress configuration, what makes the different? Keep in mind that you cannot use PHP's PATH directive via PHP-FPM right now.

#20 in reply to: ↑ 19 @tigertech
12 years ago

Replying to Whissi:

@ tigertech: If you would still vote against such a feature, could you please explain why?

I still disagree with the fundamental premise that it might sometimes be okay to send mail from a "foreign" domain name that you don't control the DNS records for.

The argument seems to be that SPF, DKIM, SpamAssassin source rules, etc. are things that some people care about, but that other people might simply choose to ignore -- that it's a policy decision made by the recipient's mail server administrator. While that's literally true, this discussion is about the problems this proposed feature might cause for the sender and end recipient, who almost certainly have no idea what that policy is.

You used "foo[@]gmail.com" as an example of an address that a WordPress user might think they "own" and is therefore reasonable to use. But look at what Google says will happen if someone does that:

http://support.google.com/mail/bin/static.py?hl=en&page=ts.cs&ts=2411000&rd=1

So Google's opinion is that "All mail sent from Gmail [addresses] should have authentication data in the message that can verify it was sent through Gmail". They don't want people sending foo[@]gmail.com mail from other servers, and they think it should be flagged as potential phishing.

Worse, if you're using a decent hosting company that has anti-spam protection on the outgoing end, the mail won't ever leave their servers in the first place. My company simply doesn't allow scripts to send mail from, say, foo[@]gmail.com. (The script can authenticate against the gmail SMTP servers if they really need to do that.) Thus, our busy mail servers almost never send any phishing mail. This is so clearly a Good Thing that eventually almost every company will block outgoing mail from foreign domain names. (In case you're thinking that our customers complain about this restriction, they almost never do, but we get lots of "thank God I switched to you; your outgoing SMTP servers are never blacklisted like my old hosting company".)

The average WordPress user will not understand any of this. Adding this proposed feature would make more mail get flagged or bounce, and and adding complexity that (on average) causes more problems for users is Wrong.

I mean, setting it via .user.php/PHP configuration or WordPress configuration, what makes the different?

You're saying "since people can break their WordPress installation by entering invalid values in php.ini, there's no extra harm in giving them a box in WordPress that they can enter the same invalid values into".

I disagree with that, though. To me, php.ini changes have an implication of "you shouldn't need to touch this unless your hosting company has set things up in an unreasonable fashion". That was pretty much the original case described here: the hosting company had their PHP mail settings in a non-working state. That's a bug, so the uncommon step of modifying php.ini was a reasonable "fix" for that unusual situation.

But the fact that someone had this problem doesn't suggest that it needs a WordPress configuration option, any more than it suggests a WordPress option is needed for many other php.ini options that might be broken. <shrug>

#21 @bpetty
12 years ago

  • Keywords close added

I think it's worth pointing out that the entire mail handling system in WordPress is pluggable, and can be replaced by something that does handle whatever extremely rare cases like this require (and this really does only appear to be required if you have no control over the mail server that is incorrectly configured in the first place).

I would like to propose closing this ticket which has only actually proposed changes to a 3rd party library anyway (and as such, this should be discussed on the PHPMailer project).

#22 @knutsp
12 years ago

  • Cc knut@… added

#23 @iandunn
12 years ago

  • Cc ian_dunn@… added

#24 @c3mdigital
12 years ago

  • Keywords reporter-feedback close removed
  • Resolution set to wontfix
  • Status changed from new to closed

3rd Party libraries should be maintained upstream whenever possible. There have been updates to the library since this ticket. See: #25014

#25 @SergeyBiryukov
12 years ago

  • Milestone Future Release deleted
Note: See TracTickets for help on using tickets.