Please remove X-Mailer from class-phpmailer

[jwz]

It is nobody's business what software I am using to send mail, or what version number it is. Providing version numbers of server-side packages to strangers is an unnecessary security exposure. With each update to WordPress, I apply this patch. I would appreciate it if you would either include this patch yourselves, or provide a hook where I can do this myself without modifying the source.

--- wp-includes/class-phpmailer.php	5 Jul 2011 20:53:19 -0000	1.3
+++ wp-includes/class-phpmailer.php	14 Dec 2011 19:43:32 -0000
@@ -1129,8 +1129,8 @@
     } else {
       $result .= sprintf("Message-ID: <%s@%s>%s", $uniq_id, $this->ServerHostname(), $this->LE);
     }
-// jwz: no.    $result .= $this->HeaderLine('X-Priority', $this->Priority);
-// jwz: no.    $result .= $this->HeaderLine('X-Mailer', 'PHPMailer (phpmailer.sourceforge.net) [version ' . $this->Version . ']');
+    $result .= $this->HeaderLine('X-Priority', $this->Priority);
+    $result .= $this->HeaderLine('X-Mailer', 'PHPMailer '.$this->Version.' (phpmailer.sourceforge.net)');
 
     if($this->ConfirmReadingTo != '') {
       $result .= $this->HeaderLine('Disposition-Notification-To', '<' . trim($this->ConfirmReadingTo) . '>');

[scribu]

Your patch is reversed and it's not in an attachment.

Also, PHPMailer is a third-party library. We prefer to leave those intact.

[dd32]

We've got a filter on wp_mail() for the PHPMailer instance, you should be able to hook into that action and alter/add things at that point I believe?

EDIT: filter: ​phpmailer_init action hook

[jwz]

Replying to dd32:

The wp_mail filter doesn't help, since that only lets me alter the arguments that WordPress passed in.

The phpmailer_init action does not let me alter the raw message produced by PHPMailer; it only lets me set some values in the $phpmailer object.

I guess I could just do $phpmailer->Version = "0.0.0" in that action to obfuscate it, but it doesn't let me do what I actually want to do: completely omit the "X-Mailer" and "X-Priority" headers.

If there was a hook inside of Send() that let me alter $header before SendmailSend, SmtpSend or MailSend were called, that would let me do what I want.

[nacin]

I have to agree with jwz. This is pretty lame.

This branding is a side effect of using a third-party library. Looking at the code, there's no way for us to overload and remove these headers without just hacking their core file. (Which I'm not objecting to.)

PHPMailer has been taken over and forked onto Google Code, so 5.1 is no longer the latest version.

See ​the announcement, ​the new repo on Google Code, ​the old repo on SourceForge.

5.2 introduces an $this->XMailer property, that when set will work in place of the PHPMailer one. X-Priority remains overridable.

[ocean90]

phpMailer 5.2: #19677

[westi]

Trunk now has PHPMailer 5.2

I have a proposed patch for PHPMailer which I will attach here and send upstream to make it easy to block the addition of the X- Headers using a plugin (or in core WordPress)

[westi]

Simple solution to block x- headers

[MattyRob]

With the code in PHPMailer 5.2 would it not be possible to pass $phpmailer->XMailer = true to over ride an X-Mailer header being sent with the current wp_mail() function plugability? If I get a chance later I'll apply the PHPMailer 5.2 patch and see if this works.

Let's not forget though that out of the box WordPress attaches the wp_generator() function to the wp_head() action and thereby brands and provides software version details in the source of every rendered page. Why are the authors of PHPMailer not allowed to do the same?

[nacin]

Replying to MattyRob:

With the code in PHPMailer 5.2 would it not be possible to pass $phpmailer->XMailer = true to over ride an X-Mailer header being sent with the current wp_mail() function plugability? If I get a chance later I'll apply the PHPMailer 5.2 patch and see if this works.

Let's not forget though that out of the box WordPress attaches the wp_generator() function to the wp_head() action and thereby brands and provides software version details in the source of every rendered page. Why are the authors of PHPMailer not allowed to do the same?

They are. But as of 5.2, it's now overridable. I'd think that the default XMailer should be the same WordPress generator.

[MattyRob]

Replying to nacin:

But as of 5.2, it's now overridable. I'd think that the default XMailer should be the same WordPress generator.

Oh yes, I like that idea. So, essentially we can maybe update wp_mail() so that X-Mailer is passes as the same output that wp_generator() produces.

I'll see if I can get a diff patch for that later today.

[MattyRob]

How about the following attachment as an update to wp_mail(). It allows X-Mailer to be set using the headers and also over rides the PHPMailer default to "WordPress" and the current version number. Same over riding applies to X-Priority. I've tested it on a basic local install and it works for me as it stands and I can successfully plug it to. Only thing I have not tried is making the 2 parameters empty but I suspect that would need a patch to the PHPMailer code so maybe we should wait for an update in the external library for that.

[MattyRob]

wp_mail() updates

[jwz]

Currently, I can remove WordPress version-branding in HTML and RSS by doing:

function no_generator() { return ''; }
add_filter('the_generator', 'no_generator');

Wouldn't it be better for php-mailer to be calling the_generator() so that all this comes from the same place?

It's also important that if the_generator() returns an empty string that the header not be emitted at all. A blank X-Mailer header is almost as good a a signature providing a version number in the first place.

As I said in my initial report, please keep in mind that the reason I'm complaining about this is that providing people with version numbers of the software that is running on remote servers is a security exposure. I'm not trying to de-brand this stuff for no reason. I'm trying to de-brand it because the first thing someone who's trying to hack your server wants to know is what it's running. The fewer identifiable clues that you provide to that, the safer you are.

I think it's a big mistake for WordPress to tell the world what version number is running by default, but at least in the case of WordPress, I can override that.

[nacin]

I think it's a big mistake for WordPress to tell the world what version number is running by default, but at least in the case of WordPress, I can override that.

Detecting a version number of web application software that is in some way publicly accessible is trivial to pin down to a version number, or at least a major branch. It could be as simple as MD5'ing CSS or JS files. For example, you're hiding it well, but I could still ascertain that you are running 3.3 on jwz.org, and should update to 3.3.1 as it was a security release. :-)

[Otto42]

Replying to jwz:

As I said in my initial report, please keep in mind that the reason I'm complaining about this is that providing people with version numbers of the software that is running on remote servers is a security exposure. I'm not trying to de-brand this stuff for no reason. I'm trying to de-brand it because the first thing someone who's trying to hack your server wants to know is what it's running. The fewer identifiable clues that you provide to that, the safer you are.

People say this a lot, but I don't think the real-world facts bear it out. If this sort of thing was a problem, my server logs wouldn't constantly show signatures from years-old attacks on ASP based systems.

People actively hacking websites don't target a specific site, in general. They download a big list of "hacks", load them into their penetration code, and mass spam them at as many sites as possible. Then they come back later and see what succeeded. Checking for a version requires an extra HTTP request, at minimum. Spamming a known attack at a site and then just seeing if it worked or not doesn't.

Sure, security testing tools have version checkers and such, because they're so easy to write. Heck, I've written a couple. They're useful for investigation, but they're useless to 99.9999% of the actual attacks being performed.

And for those people that might be targeting your site specifically, hiding your version doesn't actually help, because they're better than that. They can probably guess your version close enough to be useful just by glancing at the site, if version was at all important for security reasons.

A piece of software is either secure from the attacks performed on it, or it's not. If it's insecure, hiding the version doesn't slow down an attacker in the least. If it's secure, then having a version number doesn't help them in the least.

Edit: Note that with that said, I have no objection to making it easier to remove or change versions and such for vanity's sake. X-Mailer header included. It's just not a security issue.

[MattyRob]

Replying to jwz:

Currently, I can remove WordPress version-branding in HTML and RSS by doing:

function no_generator() { return ''; }
add_filter('the_generator', 'no_generator');

Wouldn't it be better for php-mailer to be calling the_generator() so that all this comes from the same place?

The current 'the_generator' filter echos the returned string so that would not help in the case of PHPMailer. Also, these filters ultimately link through to the ​get_the_generator() function and this function returns more than just the word WordPress with the version number appended.

The fix I attached for wp_mail() allows X-Priority and X-Mailer to be over written but currently the PHPMailer library will not accept null input for these variables (I don't think).

Of course we can fork the PHPMailer code but then there is little point of using an external library to then make custom changes to it so you have to merge in changes every time it gets updated rather than simply updating the entire file.

[jwz]

Replying to Otto42:
These are good points, but eliminating the low-hanging fruit still has merit. "You don't have to run faster than the bear, you just have to run faster than your friend."

Replying to MattyRob:
I think you're saying "there is little point to" when you mean "it would be slightly inconvenient to".

You're saying "I don't want to change PHPMailer because then I'd have to merge the changes every time I upgrade it." I'm glad you understand where I'm coming from, because that is exactly what I'm saying to you! I'm tired of hand-merging my changes to the WordPress distro every time I update it. So I reported this bug in the hopes that I'll get to stop doing that.

[Otto42]

Replying to jwz:

Replying to Otto42:
These are good points, but eliminating the low-hanging fruit still has merit. "You don't have to run faster than the bear, you just have to run faster than your friend."

Brings the "Club vs. Lojack Solutions" article to mind.

​http://web.archive.org/web/20110607161529/http://diveintomark.org/archives/2002/10/29/club_vs_lojack_solutions

Different case, but the same basic point can be made here, I think.

[MattyRob]

Replying to jwz:

Replying to MattyRob:
I think you're saying "there is little point to" when you mean "it would be slightly inconvenient to".

You're saying "I don't want to change PHPMailer because then I'd have to merge the changes every time I upgrade it." I'm glad you understand where I'm coming from, because that is exactly what I'm saying to you! I'm tired of hand-merging my changes to the WordPress distro every time I update it. So I reported this bug in the hopes that I'll get to stop doing that.

I can understand and sympathise with your frustrations, there are aspects of WordPress that I dislike and I patch these to suit my personal preference with each release too.

That said that are compelling arguments to not fork the external libraries if this can be avoided.

I only help out on here giving my time for free. I earn no income from WordPress directly. I contribute patches when I think I can help. I'm not keen on taking responsibility for continually updating and checking the PHPMailer library to keep it both current and patched for WordPress.

There is already have a slight issue that crops up periodically due to naming conventions. PHPMailer uses periods in it's file naming. (class.phpmailer.php and class.smtp.php) whereas WordPress uses hyphens (class-phpmailer.php and class-smtp.php).

The issue arises because the class.phpmailer.php file includes the class.smtp.php file so when we rename them for WordPress we have to remember to make that same filename change within the file.

Making additional changes in the hope that these are also made upstream but with no guarantee will generate work for someone. I think the best solution for WordPress is to patch wp_mail() as best we can to allow X-Priority and X-Mailer to be altered by plugins and to report (as has been done) this issue upstream to the authors of PHPMailer and hope they make the changes we would like.

[clearsite]

post removed - after a good nights sleep this idea looks quite bad :P
sorry to have bothered you.

[clearsite]

Replying to MattyRob:

There is already have a slight issue that crops up periodically due to naming conventions. PHPMailer uses periods in it's file naming. (class.phpmailer.php and class.smtp.php) whereas WordPress uses hyphens (class-phpmailer.php and class-smtp.php).

The issue arises because the class.phpmailer.php file includes the class.smtp.php file so when we rename them for WordPress we have to remember to make that same filename change within the file.

There is a trivial way to never have to change the filenames again, but... it will create some clutter;
Just create class-phpmailer.php and have it read "require_once 'class.phpmailer.php'; "

Simple, but not very elegant and certainly off-topic :P

[chriscct7]

There isn't that much interest in removing it. Closing as wontfix.

[Scott_N]

FWIW... I was able to remove the X-mailer headers from all outgoing emails by adding the following code to my theme's functions.php file:

add_action( 'phpmailer_init', 'disable_xmailer' );
function disable_xmailer( $phpmailer ) {
	$phpmailer->XMailer = ' ';
}

(Note there is a single space character between the quotes.)

[piskvorky]

There isn't that much interest in removing it. Closing as wontfix.

As judged by whom? Such headers are a poor practice, for the exact reasons @jwz listed.

We're also quite keen to get rid of this (irrelevant, risk-adding) header.