$_SERVER['SERVER_NAME'] not a reliable when generating email host names

[layotte]

For quite some time I have been having an issue with my comment notifications. The From address has been wordpress@_. I haven't paid much attention to this, but I had some spare time and decided to pursue the issue. Here it is...

I am running WPMS w/ domains (latest stable) with Nginx and PHP5-FPM. In WordPress the comment notifications from email addresses are being generated using $_SERVER['SERVER_NAME'] to get the current site's domain name.

e.g.

$wp_email = 'wordpress@' . preg_replace('#^www\.#', '', strtolower($_SERVER['SERVER_NAME']));

However, because of my environment my Nginx config for my site has the server_name set to "_" (underscore). Which is a catchall -- ​http://nginx.org/en/docs/http/server_names.html.

Site config in Nginx:

# Redirect everything to the main site.
server {
        listen [::]:80 default_server;
        listen [::]:443 ssl;

        ssl_certificate         /etc/nginx/ssl.crt/xxx.com.2012.crt;
        ssl_certificate_key     /etc/nginx/ssl.key/xxx.com.2012.key;

        server_name _;
        root /var/www/xxx.com;
        access_log /var/log/nginx/xxx.com.access.log;
        error_log /var/log/nginx/xxx.com.error.log;
        client_max_body_size 100m;

        if ($http_host = "www.xxx.com") {
                rewrite ^ http://xxx.com$request_uri permanent;
        }

        include global/restrictions.conf;

        # Additional rules go here.

        include global/wordpress-ms.conf;

}

The default fastcgi_params has this set:

fastcgi_param   SERVER_NAME             $server_name;

Thus, $_SERVER['SERVER_NAME'] is outputting "_" (underscore).

I propose we move away from $_SERVER['SERVER_NAME'] when generating the from email addresses and use the available $current_site global object which stores the domain variable ($current_site->domain). I have implemented this change on my own site and have included the patch here.

In the meantime, anyone else facing this problem can change their fastcgi_param to $host instead of $server_name. In my opinion, not the best solution, but it works for now.

[layotte]

Replace $_SERVERSERVER_NAME? w/ $current_site->domain

[SergeyBiryukov]

Related: #16805

[nacin]

nginx has an odd quirk that, given:

server_name example.com blog.example.com microsite.example.com;

And:

fastcgi_param SERVER_NAME $server_name;

When accessing microsite.example.com, SERVER_NAME will be set to example.com. This is akin to always using ServerName in Apache, rather than using ServerAlias (which is what happens).

I agree that SERVER_NAME should not be used, as depending on the setup it is not going to match HTTP_HOST. That said, if you are using a catchall like _ (it is actually meaningless, any special character will do), you should consider doing something like set $name X in your server block to ensure $server_name is what you want it to be, or actuall change the fastcgi_param call to set SERVER_NAME to $host. That will fix your issues.

So, this isn't a duplicate of #16805 — rather, we should investigate and consider a switch to HTTP_HOST here.

[layotte]

Would HTTP_HOST be better than $current_site->domain?

[sreedoap]

I wanted to note my use case for this which caused me to run into this issue. I am calling wp_install from a command line script I made which is ran for our users who launch a WordPress site, so HTTP_HOST and SERVER_NAME were not set for me. I manually set it in my script and that fixed my issue but it would probably be wise for the wp_mail function to have a fallback if it is unable to get a valid value from HTTP_HOST or SERVER_NAME so emails don't appear to come from an "unknown sender".

Replying to layotte:

For quite some time I have been having an issue with my comment notifications. The From address has been wordpress@_. I haven't paid much attention to this, but I had some spare time and decided to pursue the issue. Here it is...

I am running WPMS w/ domains (latest stable) with Nginx and PHP5-FPM. In WordPress the comment notifications from email addresses are being generated using $_SERVER['SERVER_NAME'] to get the current site's domain name.

e.g.

$wp_email = 'wordpress@' . preg_replace('#^www\.#', '', strtolower($_SERVER['SERVER_NAME']));

However, because of my environment my Nginx config for my site has the server_name set to "_" (underscore). Which is a catchall -- ​http://nginx.org/en/docs/http/server_names.html.

Site config in Nginx:

# Redirect everything to the main site.
server {
        listen [::]:80 default_server;
        listen [::]:443 ssl;

        ssl_certificate         /etc/nginx/ssl.crt/xxx.com.2012.crt;
        ssl_certificate_key     /etc/nginx/ssl.key/xxx.com.2012.key;

        server_name _;
        root /var/www/xxx.com;
        access_log /var/log/nginx/xxx.com.access.log;
        error_log /var/log/nginx/xxx.com.error.log;
        client_max_body_size 100m;

        if ($http_host = "www.xxx.com") {
                rewrite ^ http://xxx.com$request_uri permanent;
        }

        include global/restrictions.conf;

        # Additional rules go here.

        include global/wordpress-ms.conf;

}

The default fastcgi_params has this set:

fastcgi_param   SERVER_NAME             $server_name;

Thus, $_SERVER['SERVER_NAME'] is outputting "_" (underscore).

I propose we move away from $_SERVER['SERVER_NAME'] when generating the from email addresses and use the available $current_site global object which stores the domain variable ($current_site->domain). I have implemented this change on my own site and have included the patch here.

In the meantime, anyone else facing this problem can change their fastcgi_param to $host instead of $server_name. In my opinion, not the best solution, but it works for now.

[SergeyBiryukov]

#27811 was marked as a duplicate.

[szepe.viktor]

Automatic updater sends an email but SERVER_NAME is not defined when wp-cron starts as a real Linux cron job.

Please uset get_option('site_url') as a fallback to determine the hostname.

[jesin]

Refreshed server_name.diff and corrected minor errors

[jesin]

Tested 25239.diff on a multisite Nginx environment with the following code.

add_filter( 'pre_site_option_admin_email', function() {
	return '';
});
wpmu_signup_user_notification( 'jesin', 'my@email.com', 'abcd' );
wpmu_welcome_notification( 2, 1, 'password', 'Hello World' );
wpmu_welcome_user_notification( 1, 'password' );
wpmu_signup_blog_notification( 'example.com', '/', 'Hello World', 'jesin', 'my@email.com', 'abcd' );
wp_mail( 'my@email.com', 'Subject', 'A message without From: header' );

This patch uses home_url() for single site and $current_blog->domain for multisite inside functions wp_mail() and wp_notify_postauthor() because home_url() returns the subdomain even if domain mapping is configured.

[kitchin]

Replying to layotte:

I propose we move away from $_SERVER['SERVER_NAME'] when generating the from email addresses and use the available $current_site global object which stores the domain variable ($current_site->domain).

I suggest also moving away from $_SERVER['HTTP_HOST']. At first glance, HTTP_HOST seems worse because it's more closely derived from user input, but actually both can be buggy depending on the web server. Apache has had bugs. See ​http://stackoverflow.com/questions/2297403/http-host-vs-server-name for a pretty detailed write-up of SERVER_NAME vs. HTTP_HOST.

[SergeyBiryukov]

#33698 was marked as a duplicate.

[SergeyBiryukov]

#16805 was marked as a duplicate.

[ocean90]

#37548 was marked as a duplicate.

[mensmaximus]

Can we fix this for 4.7 please?

There are several places in WordPress where we rely on $_SERVER['SERVER_NAME'] or $_SERVER['HTTP_HOST']. While I personally never trust in $_SERVER['SERVER_NAME'] I understand some feel uncomfortable with $_SERVER['HTTP_HOST'] either.

Given the fact that home_url and blog_id are always set and can be trusted, introducing a new function might be a solution:

function get_current_domain() {
        return parse_url( get_home_url( get_current_blog_id() ), PHP_URL_HOST );
}

The function name follows the syntax from other get_ functions in WordPress. It's name says what it does. It relies on WordPress core functions and settings that always exist. It works with PHP down to 4.0.

So instead of using $_SERVER['SERVER_NAME'] or $_SERVER['HTTP_HOST'] you could use get_current_domain().

[mensmaximus]

To make the function more flexible and to strip the subdomain for email addresses we could pass some arguments:

function get_current_domain( $strip_subdomain = true, $blog_id = '' ) {
        $blog_id = empty( $blog_id ) ? get_current_blog_id() : $blog_id;
        $domain = parse_url( get_home_url( $blog_id ), PHP_URL_HOST );
        if ( true === $strip_subdomain ) {
                $domain_parts = explode ( ".", $domain );
                array_shift( $domain_parts );
                $domain = implode( '.', $domain_parts );
        }
        return $domain;
}

[mt8.biz]

From WordPress4.6, wp_mail uses phpmailer->setfrom, ”wordpress@_” is always results error.

https://core.trac.wordpress.org/changeset/38287

error message is:

Fatal error: Uncaught exception 'phpmailerException' with message 'Invalid address: wordpress@_' in /var/www/vhosts/i-XXXXXXXX/wp-includes/class-phpmailer.php:946
Stack trace:
#0 /var/www/wordpress/wp-includes/pluggable.php(352): PHPMailer->setFrom('wordpress@_', 'WordPress')
#1 /var/www/wordpress/wp-includes/pluggable.php(1726): wp_mail('hogehoge@...', '[SITE NAME]')
#2 /var/www/wordpress/wp-includes/user.php(2350): wp_new_user_notification(35, NULL, 'both')
#3 [internal function]: wp_send_new_user_notifications(35, 'both')
#4 /var/www/wordpress/wp-includes/plugin.php(524): call_user_func_array('wp_send_new_use...', Array)
#5 /var/www/wordpress/wp-admin/includes/user.php(196): do_action('edit_user_creat...', 35, 'both')
#6 /var/www/wordpress/wp-admin/user-new.php(116): edit_user()
#7 {main} thrown in /var/www/wordpress/wp-includes/class-phpmailer.php on line 946

[Grzegorz.Janoszka]

As mentioned earlier - it completely doesn't work when system cron is used. Please fix it.

[mt8.biz]

Replying to Grzegorz.Janoszka:

As mentioned earlier - it completely doesn't work when system cron is used. Please fix it.

How to reproduce?

[Grzegorz.Janoszka]

Replying to mt8.biz:

As mentioned earlier - it completely doesn't work when system cron is used. Please fix it.

How to reproduce?

Wordpress + nginx + system cron sending emails.

Not sure if nginx is required, when you use system cron the variables coming from http daemon will not be set. How do you expect to get anything from $_SERVER['SERVER_NAME'] or $_SERVER['HTTP_HOST'] when you just call the php script from cron (php wp-cron.php)?

[mt8.biz]

Replying to Grzegorz.Janoszka:

Replying to mt8.biz:

As mentioned earlier - it completely doesn't work when system cron is used. Please fix it.

How to reproduce?

Wordpress + nginx + system cron sending emails.

Not sure if nginx is required, when you use system cron the variables coming from http daemon will not be set. How do you expect to get anything from $_SERVER['SERVER_NAME'] or $_SERVER['HTTP_HOST'] when you just call the php script from cron (php wp-cron.php)?

Should use the DB settings instead of using the $ _SEREVER I think

[mensmaximus]

Replying to mt8.biz:

Should use the DB settings instead of using the $ _SEREVER I think

I already posted a suggestion to use the home_url: https://core.trac.wordpress.org/ticket/25239#comment:22

[mt8.biz]

Replying to mensmaximus:

Replying to mt8.biz:

Should use the DB settings instead of using the $ _SEREVER I think

I already posted a suggestion to use the home_url: https://core.trac.wordpress.org/ticket/25239#comment:22

yes. I khow.

[mt8.biz]

This is a fatal problem from ver4.6. I think it should be fix quick.

[cbutlerjr]

@jesin's patch is a little more thorough in addressing the issue - it also covers the wp_notify_postauthor() function as well as ms-functions.php. But it probably needs a refresh.
https://core.trac.wordpress.org/attachment/ticket/25239/25239.diff

[jesin]

Refreshed 25239.diff​

[dd32]

An issue affecting 4.6 installs is fixed in 4.6.1, see #37736

[Grzegorz.Janoszka]

Replying to dd32:

An issue affecting 4.6 installs is fixed in 4.6.1, see #37736

dd32: the one we are discussing here is completely different and unrelated. It is about using HTTP variables when they are not available (like when calling wp-cron script from system cron.

[ocean90]

#37945 was marked as a duplicate.

[BjornW]

@ocean90 & @dd32 is there a reason for not accepting @jesin's patch?

[neodjandre]

I am facing the same problem using:

Nginx + Postfix + Real Cron Jobs

I am receiving these errors:

Error message
phpmailerException: Uncaught exception 'phpmailerException' with message 'Invalid address: wordpress@' in /var/www/html/wp-includes/class-phpmailer.php:946

Stack trace
in PHPMailer::setFrom called at /var/www/html/wp-includes/pluggable.php (352)
in wp_mail called at /var/www/html/wp-includes/user.php (1841)
in wp_update_user called at /var/www/html/wp-admin/includes/user.php (182)
in edit_user called at /var/www/html/wp-admin/user-edit.php (147)

[BjornW]

In my case I've fixed this with this ​small plugin. It uses the admin user's email address as a From address thus enforcing a working email address. For now, this is a workable solution for me at least until @jesin's patch or someone else's is accepted in WordPress. Hope this helps.

Replying to neodjandre:

I am facing the same problem using:

Nginx + Postfix + Real Cron Jobs

I am receiving these errors:

Error message
phpmailerException: Uncaught exception 'phpmailerException' with message 'Invalid address: wordpress@' in /var/www/html/wp-includes/class-phpmailer.php:946

Stack trace
in PHPMailer::setFrom called at /var/www/html/wp-includes/pluggable.php (352)
in wp_mail called at /var/www/html/wp-includes/user.php (1841)
in wp_update_user called at /var/www/html/wp-admin/includes/user.php (182)
in edit_user called at /var/www/html/wp-admin/user-edit.php (147)

[Ipstenu]

FYI, since we changed how phpMailer validates emails, this has caused a previously silent error in wp-cli to bomb out.

​https://github.com/wp-cli/wp-cli/issues/3374

the tl;dr version is that because the email is being passed as wordpress@ without a domain (since php CLI can't grab that variable), and phpmailer now flags that as an error, no one can reset passwords on command line unless they use the --url parameter to force the domain in.

The change in 4.6 (and 4.6.1) highlights a problem that had been flying under the radar for a long time, I think.

[ocean90]

#38161 was marked as a duplicate.

[danielbachhuber]

At the very least, could we get a try... catch around the thrown Exception?

[Ipstenu]

From #38161

Using something like substr( home_url( '', 'http'),7) might be a suitable substitute when $_SERVER is unavailable. (If there's a better way to get a site's hostname than slicing up home_url() that of course would work too.)

We'd want to use parse_url() I think, since https is everywhere. Also some people use WWW and some don't :/

[desmith]

This patch combines @jesin's "replace it in a bunch of spots" and @mensmaximus's "add a new function for this" techniques. I omitted the parameters for get_current_domain() because there's only one current domain at any given time. First patch, probably did everything wrong, et cetera.

[desmith]

Please ignore the above patch (which doesn't fix all the cases of this that I've encountered), in lieu of another one coming shortly. Apologies.

[desmith]

Okay, let's try this again.

This patch adds a new function in pluggable.php, get_current_domain(). Does what it says on the tin, returns the current blog domain name, with leading 'www.' stripped. It also modifies a few places where emails are sent, to use that function instead of $_SERVER['SERVER_NAME'], notably in the password reset email-send, which fixes the issue with wp-cli password reset emails.

There is a possible side effect. Since I replaced $_SERVER['SERVER_NAME'] in several email-related places, anyone that's depending on their emails' From: headers looking like support@… instead of support@theirsite will be disappointed. I could un-patch those instances, of course, but I think this behavior is cleaner overall.

[joemcgill]

Replacing $_SERVER['SERVER_NAME'] with output from a helper function in these instances seems like a good idea to me and could be useful outside of instances where we're building email addresses. I question whether this should be a pluggable function, though. This function feels more like get_site_url() or get_home_url() which live in wp-includes/link-template.php.

I also wonder if it was an oversight that none of the multisite functions were stripping www subdomain prefixes or if that was intentionally allowed? Either way, 25239.4.patch​ has a redundant preg_replace() call if that method is used inside the helper function.

[jdgrimes]

Replying to joemcgill:

I question whether this should be a pluggable function, though.

Yeah, I think that the consensus is that pluggable functions were a mistake, and filter and action hooks should be used to provide overridability/customizability. So no new pluggable functions are being added, as far as I know.

So the function should be moved to a different file as you've suggested, and we should consider the merits of adding a filter on the value that it returns.

[desmith]

As suggested, I've moved the new function to a more-appropriate place, and added an apply_filters call.

@joemcgill , you were right in that I overlooked a preg_replace call; thanks for the catch.

The question of whether we should remove leading 'www.' in names here is still open. It is a change, and while I'd argue it's a change for the better, it's a pretty weak argument.

[mensmaximus]

Replying to desmith:

The question of whether we should remove leading 'www.' in names here is still open. It is a change, and while I'd argue it's a change for the better, it's a pretty weak argument.

Please ounce again take a look at my suggestion: https://core.trac.wordpress.org/ticket/25239#comment:22

As you can see I have addressed the subdomain issue by passing a parameter to the function. I also passed a blog id parameter to make the function more universal.

[Ipstenu]

@joemcgill - Probably an oversight or an 'It didn't break, leave it alone.' which actually applies to this change as a whole ;)

Leading www's should be removed when we're talking about _email_ however. You don't email support@… after all. :) (Hopefully a better argument since part of the point of this ticket is making VALID email address)

[joemcgill]

@Ipstenu thanks for pointing out that those are for the From: emails. Even so, your point about "leave it alone" stands, so I wonder if we should only use home_url() here as a fallback whenever $_SERVER['SERVER_NAME'] isn't set—which is the reason for this ticket in the first place.

@mensmaximus Thanks for the reminder about your approach. If I understand your thinking, making this function more flexible would also allow it to be used instead of $_SERVER['HTTP_HOST'] as well, correct? If so, I worry about there becoming overlap/confusion between this function and already existing functions, like get_site_url(), get_home_url(), etc.

Is there a needed use case for a function that does any more than checks to see if $_SERVER['SERVER_NAME'] is set, and if not, returns home_url() (with a www subdomain stripped, if applicable)? If not, it may be better to keep the scope of this narrowly defined as a wrapper around $_SERVER['SERVER_NAME'] and to reinforce its purpose in the function name—so maybe something like wp_server_name() or get_server_name() instead.

[desmith]

Let's reduce the scope a bit, then. Latest version just has a function called get_email_domain() that returns the domain name, leading www. stripped, and that's it. Function name is a bit more clear, at least. All the places it's being used are related to email, so the function name and its use make a bit more sense.

If we want to add an if{} to only act if $_SERVER['SERVER_NAME'] is unset, easy enough to do, but I'm not sure it's necessary. Thoughts/opinions?

[riasat]

WP v4.6.1

My nginx conf server block:

server {
	listen         80;
	server_name    ~^(?<subdomain>.+?)\.(?<domain>.+)$;
	root  /var/www/$subdomain;
	include conf.d/server_block_common.cnf;
}

This is what I always get:

Fatal error:  Uncaught phpmailerException: Invalid address: wordpress@~^(?<subdomain>.+?)\\.(?<domain>.+)$ in 

I know I can fix it by using "wp_mail_from" but I still don't get it why you can not simply default to "wordpress@…" or even just give an OPTION in settings to set the SEND FROM field. It is ridiculous that people have to resort to code if they want to change wordpress portion of the SEND FROM field.

[dd32]

Replying to riasat:

server {
	listen         80;
	server_name    ~^(?<subdomain>.+?)\.(?<domain>.+)$;
	root  /var/www/$subdomain;
	include conf.d/server_block_common.cnf;
}

This is what I always get:

Fatal error:  Uncaught phpmailerException: Invalid address: wordpress@~^(?<subdomain>.+?)\\.(?<domain>.+)$ in 

@riasat Just focusing on your error here, rather than the ticket - Looks like you've got a error in your nginx config there, although that should work for matching (and appears to be), the matched name isn't be passed to PHP, instead it's passing the literal regex through.
You need to correct the value being passed to PHP through fastcgi_param SERVER_NAME ... in your nginx config, something like ​http://serverfault.com/questions/650560/nginx-regex-vhost-pattern-ends-up-as-php-server-name should set your on your way.

[derekakelly]

I am also getting this bug, although it's only happening on my "add-on domain". My main website also uses the same configuration but does not have this error. I am on a Cloud VPS hosting account. just thought it was odd so I'd point it out. This is my cronjob command

/usr/local/php56/bin/php-cli /home/xxxxxx/public_html/xxxxxxxx.com/wp-cron.php

This is also a new error, up until a week or two ago, i'd never seen it before.

Thanks!

[dd32]

In 39655:

Mail: Ensure that any phpmailerException exceptions generated by setFrom() are caught to avoid PHP Fatal errors.

This change avoids a PHP fatal error that can be encountered when the specified (or generated) source email is an invalid address, such as wordpress@_, it makes no effort to set a valid source, only avoid the fatal error.

See #25239 for correcting the email address.
Fixes #39360.

[swissspidy]

#40259 was marked as a duplicate.

[lilmike]

Hi,
hJust a quick recap of the ticket I submitted this morning that was a duplicate:
I have a postfix that's set to send emails from certain domains (including my server's fqdn) through an email deliverability service (in the case of the server's fqdn to stop my server notifications to me -- crons, etc -- from being marked as junk or dropped entirely). However, when WordPress sends an email, the envelope-from gets set to 'http', and then, by postfix, to http@<fqdn>. This causes postfix to try and relay through the afore-mentioned provider... only to get a bounce because the from: is not a configured sending domain. This would be a helpful fix, even though I have a plugin currently installed and activated in the sites with the problem that cause it to actually set the right envelope-from.
All this rambling to say -- please fix, it's certainly been a while since this ticket was opened :-(.
-Michael.

[tsimmons]

This seems to have escalated due to the release of ​https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-8295 and ​https://exploitbox.io/vuln/WordPress-Exploit-4-7-Unauth-Password-Reset-0day-CVE-2017-8295.html.

Allowing user-supplied input without check or verification is a terrible thing.

For now I'm using @BjornW 's plugin ​https://wordpress.org/plugins/admin-email-as-from-address/#description to mitigate the exposure, but we really should be pulling this information from the site_url and an officially sanctioned patch should be issued.

[tsimmons]

Replaces domain part with the typical email domain for a site, which includes the last domain part + TLD

[tsimmons]

(Reuploading to use proper SVN diff) Replaces domain part with the typical email domain for a site, which includes the last domain part + TLD ... for example, if the site is blog.wordpress.org the email domain becomes wordpress.org

[kitchin]

@tsimmons, Have not reviewed all this bug, but looks like you would be sending a lot of mail to wordpress@co.uk. Does WP use something like the public suffix list? ​https://publicsuffix.org/

[robert.peake]

@tsimmons @kitchin Indeed, seems like it might be most backwards-compatible with the current solution to just do:

<?php
...
if ( !isset( $from_email ) ) {
    // Get the site domain and get rid of www.
    $sitename = strtolower( parse_url( get_home_url( get_current_blog_id() ), PHP_URL_HOST ) );
    if ( substr( $sitename, 0, 4 ) == 'www.' ) {
        $sitename = substr( $sitename, 4 );
    }

    $from_email = 'wordpress@' . $sitename;
...

[tsimmons]

Replying to robert.peake:

@tsimmons @kitchin Indeed, seems like it might be most backwards-compatible with the current solution to just do:

<?php
...
if ( !isset( $from_email ) ) {
    // Get the site domain and get rid of www.
    $sitename = strtolower( parse_url( get_home_url( get_current_blog_id() ), PHP_URL_HOST ) );
    if ( substr( $sitename, 0, 4 ) == 'www.' ) {
        $sitename = substr( $sitename, 4 );
    }

    $from_email = 'wordpress@' . $sitename;
...

@kitchin Y'all are exactly right. There are a vast number of domain suffixes that are not taken into account with my patch. @robert.peake 's fix is superior; I think I'm going to stick with the plugin from @BjornW for now. Probably the only way to properly handle this is to utilize something like ​https://github.com/jeremykendall/php-domain-parser in the domain parsing.

[DavidAnderson]

Why is WP trying to come up with an email address at all, rather than using the one in 'General Settings'? The plugin ​https://wordpress.org/plugins/admin-email-as-from-address/ has been mentioned - but why isn't this the existing behaviour?

[tsimmons]

Replying to DavidAnderson:

Why is WP trying to come up with an email address at all, rather than using the one in 'General Settings'? The plugin ​https://wordpress.org/plugins/admin-email-as-from-address/ has been mentioned - but why isn't this the existing behaviour?

My answer would be because system generated messages (like password resets or notifications) aren't really from the blog author, but from the backend. I use the aforementioned plugin and use wordpress@… for admin messages. Of course someone else may have another opinion.

[cloudstek]

I see I'm a bit late to the party lol, doh!

@tsimmons interesting thought about removing the subdomain(s) from the email domain, didn't think of that. Though one could argue it might be desired for some sites to include the subdomain in the email domain. I'm not that familiar with multisite but wouldn't that be one of the cases having the subdomain in the from header is actually desired?

With all the patches supplied, can I assume this will be fixed soon asap?

[Ipstenu]

The point @tsimmons brings up, about emails like password resets that shouldn't come from the admin, actually touches back on why this is considered a security issue. Admins shouldn't get the bounced email with the link to password reset.

In addition, some servers block emails sent FROM anyone but the domain WP is installed on. So your personal email of foobardancingfox@… wouldn't get the messages :( Oh and lets not think about the spoofing of emailing from gmail while actually not being from gmail. Fast track to being blacklisted :(

[DavidAnderson]

@tsimmons I'm thinking, having read this ticket, that I wonder if I'm alone in thinking that it's not clear what this email address is *for*. Taking up your comment about the person/backend distinction... since one can't have an email conversation with a backend, since it isn't human, then presumably the idea is that it's effectively a "noreply@" address? i.e. An address that isn't really intended to get replies, and is just for aesthetics? (Of course, a site admin can set it up so that he *does* get replies (or he may have a catch-all) - but then he may as well just use the admin address anyway in that case).

@Ipstenu I don't follow all of this. A few functions in wp-includes/ms-functions.php already do use the admin email as a From: address (I'm wondering whether there's a design in which emails do/don't, or if it's just a accumulation of unplanned independent decisions). Also, it can't be dangerous for the site admin see password reset requests, can it? He can already reset as many passwords as he likes, and/or set up a wordpress@ email address to the replies anyway (or may already have a catch-all).

[Ipstenu]

I don't know why we aren't consistent about email-fromness. I thought that we were fairly consistent in that if it's a message from the system (updates, password links etc) they came from wordpress@ across the board. A quick look at ms-functions and it appears emails that should have a 'contact' back (like 'you've got a new blog!' on multisite), where there's a reasonable expectation to know who mom is are sent from the network admin.

Also, it can't be dangerous for the site admin see password reset requests, can it? He can already reset as many passwords as he likes, and/or set up a wordpress@ email address to the replies anyway (or may already have a catch-all).

It's a higher risk. Remember, risk isn't a 1/0 switch. There are gradients. Most people don't make a wordpress@ email, or even a catch-all. But also most people don't use 2FA or good passwords on their email (see Google and Yahoo). It's possible for someone to snipe emails and get your passwords/resets without raising a red flag like "Hey, I (the admin) didn't ask to rent my password..."

In short, it's not dangerous for the admin to have your password. It's dangerous for the uneducated and non-security conscious admin to clear-text read your bounced messages in a coffee shop :/ (Plus I bet the bounces would confuse a lot of people...)

Email's not very secure, is basically my point :) Or rather, people USING email aren't very secure in general, so if using a generic wordpress@ will protect more people at minimal cost, then we probably should do that.

I do find it interesting we have $admin_email = 'support@' . $_SERVER['SERVER_NAME']; in there as a fallback if there's no admin.

[cloudstek]

Replying to Ipstenu:

I don't know why we aren't consistent about email-fromness. I thought that we were fairly consistent in that if it's a message from the system (updates, password links etc) they came from wordpress@ across the board. A quick look at ms-functions and it appears emails that should have a 'contact' back (like 'you've got a new blog!' on multisite), where there's a reasonable expectation to know who mom is are sent from the network admin.

Also, it can't be dangerous for the site admin see password reset requests, can it? He can already reset as many passwords as he likes, and/or set up a wordpress@ email address to the replies anyway (or may already have a catch-all).

It's a higher risk. Remember, risk isn't a 1/0 switch. There are gradients. Most people don't make a wordpress@ email, or even a catch-all. But also most people don't use 2FA or good passwords on their email (see Google and Yahoo). It's possible for someone to snipe emails and get your passwords/resets without raising a red flag like "Hey, I (the admin) didn't ask to rent my password..."

In short, it's not dangerous for the admin to have your password. It's dangerous for the uneducated and non-security conscious admin to clear-text read your bounced messages in a coffee shop :/ (Plus I bet the bounces would confuse a lot of people...)

Email's not very secure, is basically my point :) Or rather, people USING email aren't very secure in general, so if using a generic wordpress@ will protect more people at minimal cost, then we probably should do that.

I do find it interesting we have $admin_email = 'support@' . $_SERVER['SERVER_NAME']; in there as a fallback if there's no admin.

One reason to send email from the admin user instead of a non-existing wordpress@ email is that it will likely fail sender verification when it's enabled on the receiving server and thus be rejected and not delivered.

Also to prevent people from replying to it you can set the Reply-To header to noreply@ or if you like wordpress@. Though I find it more likely people won't create a noreply@ address than a wordpress@ address. Therefore I'd prefer the use of noreply@ as it makes clear you're not supposed to reply. We wouldn't need wordpress@ for aesthetic purposes as the Reply-To field is likely to be hidden by default in most email clients.

Preventing bounces is a little harder but according to Wikipedia there are a number of headers that can be sent in order to direct bounce emails to a specific (non-existing) address: ​https://en.wikipedia.org/wiki/Bounce_address.

[DavidAnderson]

@cloudstek @Ipstenu If the intention of the "wordpress@" address is that replies are meant to be as hard/invisible as possible, then there might be a case for a no-reply@… address that is a log-less black-hole (and people who don't trust wordpress.org (which makes no sense, unless they also turn off all manner of other things) can use a plugin or filter to set their own). The number of people with catch-alls must be a decent number. So, if this is the case, I'd say that the logical conclusion is a universally-the-same address like this.

@cloudstek Sender verification uses the SMTP envelope From, rather than the header From:. i.e. WordPress has been relying on these two being different (whether intentionally or not) to get these emails through sender validity checks. Which is to say, though, that it's basically just ornamental, and we could use "bob-the-giant@…" (that's why, despite using an address that often doesn't exist, they still get through - though, this is just the norm - I have no stats on how many mail servers will also verify the header From: as well as the SMTP envelope From). That point is also relevant to @Ipstenu's comment about email delivery restrictions on some servers - these are normally looking at the SMTP envelope. i.e. To the extent that the admin address can't be used because of server restrictions, to the same extent a wordpress@ address can't be used because of sender verification.

[tsimmons]

Replying to kitchin:

@tsimmons, Have not reviewed all this bug, but looks like you would be sending a lot of mail to wordpress@co.uk. Does WP use something like the public suffix list? ​https://publicsuffix.org/

I don't think so, and probably really shouldn't. It really should just be a configurable option (from address for admin messages) which the plugin provides. Automatically deriving from user supplied input DIRECTLY is bad, though, as the present code does.

[tsimmons]

Replying to DavidAnderson:

since it isn't human, then presumably the idea is that it's effectively a "noreply@" address?

Exactly. it should be left up to the user to define what address they want system messages to come from, a simple configuration option, that defaults to the admin's address if omitted, IMO.

[DavidAnderson]

Replying to tsimmons:

I agree with you. The default WP behaviour currently is to invent a new email address with an associated pseudo-persona for the site, without the site or domain owner's involvement. I can't see what the up-sides to that are.

[tsimmons]

Replying to DavidAnderson:

Replying to tsimmons:

I agree with you. The default WP behaviour currently is to invent a new email address with an associated pseudo-persona for the site, without the site or domain owner's involvement. I can't see what the up-sides to that are.

Yup. The security paranoid (I am one) should implement the plugin above until core is patched. The patch I supplied should not be used because it does a poor job of detecting TLD/public suffixes.

[kitchin]

Empty return-path is the age-old way of preventing bounces. Bounces themselves set an empty return-path, as do "out of the office" auto-responders. Return-path is part of the SMTP envelope, unlike message headers like return-to. Sorry if this information is bug spam for most people, but maybe it's not.

We could both try to stop bounces AND use the owner email address, which we can assume has been affirmatively set up during install. It won't catch everything, due to the variety of setups, pluggables, filters, etc. And there might be legacy blowback. Imagine someone has written a filter that detects wordpress@! Too bad.

Could send a security email advisory to the owner address during the upgrade.

[tsimmons]

Replying to kitchin:

Empty return-path is the age-old way of preventing bounces. Bounces themselves set an empty return-path, as do "out of the office" auto-responders. Return-path is part of the SMTP envelope, unlike message headers like return-to. Sorry if this information is bug spam for most people, but maybe it's not.

We could both try to stop bounces AND use the owner email address, which we can assume has been affirmatively set up during install. It won't catch everything, due to the variety of setups, pluggables, filters, etc. And there might be legacy blowback. Imagine someone has written a filter that detects wordpress@! Too bad.

Could send a security email advisory to the owner address during the upgrade.

Personally I'd prefer an option to set the from-address, so a user could use their own email or a null-routed noreply@ if desired. Or even a user-supplied account that can receive replies. It's up to the user to choose, just use reasonable defaults for the less tech-savvy.

[Grzegorz.Janoszka]

@DavidAnderson: There are servers on internet that verify the envelope-from. I used to send emails from non-existing wordpress@…, but after a few servers that refused to accept emails, I changed to an existing email (actually the admin email).

[pigdog234]

Hi,

Seems like a very short term way to address the CVE is to just drop something like the following in:

add_filter( 'wp_mail_from', function( $email ) {
     return 'wordpress@example.com';
});

Right?

[tsimmons]

Replying to pigdog234:

Hi,

Seems like a very short term way to address the CVE is to just drop something like the following in:

add_filter( 'wp_mail_from', function( $email ) {
     return 'wordpress@example.com';
});

Right?

The problem with this approach is it doesn't work with a multi-domain site where the from address should be domain-specific.

[pigdog234]

Indeed. And so in a multidomain environment it seems to me that exposing $sitename for this purpose as a configuration element is useful.

BTW, it might be possible to test an outbound email by creating a little workflow along the following lines:

1. User enters the address
2. Address set to unverified (do not use yet)
3. Email is sent to a drop box on some picky email gateway like Gmail
4. Local installation sets an event to check validation with WP.com (or some service)
5. WP.com (or some service) has a little process that pulls email from the box and validates DKIM/SPF and that the message got through
6. Local installation polls WP to validate the address.

It can't happen all at once because of grey listing effects (that's a fair amount of work). It also may be more trouble than it's worth. At the end of the day it would be nice if notifications happened in a WP app.

[pigdog234]

Just for clarity, whose responsibility is it to police SERVER_NAME in a multi-domain configuration? Should it be Apache's or Wordpress'? Who knows what the valid values should be?

[BjornW]

Maybe I've missed something in this thread, but what are the arguments against having an option for setting the email address with a sensible default?

[rawrly]

Because the ticket and this case is a bit old, and has begun to take on a lots of side-issues related to the same bit of code I wanted to summarize the issue(s) at hand and repeat the currently open questions and solutions provided i a concise manner for those just catching up:

Original ticket's focus:

Not always does $_SERVER['SERVER_NAME'] get passed down to PHP from the web-server environment. This causes emails to be fail to send due to invalid email address From address fields.

This affects some nginx server configurations as well as wp-cli and sometimes via Cron tasks. Leading to emails being unable to be sent when they are related to the function of the request.

The later reported vulnerability:

A pseudo-vulnerability was released related to this portion of WP core code in early 2017, where in specific and unlikely server configurations this could be used to intercept password reset tokens.

Where this bug stands is two parts:

There is an inconvenient choice for the FQDN after @ when generating the From address in emails, where in some situations it will cause emails to not be sent and the application to not function correctly.

There is an unlikely but possible vulnerability also related to this same portion of code, and the fact it accepts browser input (the value from the Host header field) for the value of the email address to send emails From.

Proposed Patches summary:

Most patches have been focused on how to get a FQDN from somewhere other that $_SERVER['SERVER_NAME']

"server_name.diff" does a replacement of all $_SERVER['SERVER_NAME'] to now use $current_site->domain

"25239.patch" creates a function get_email_domain() which uses preg_replace to extract the domain name from get_home_url( get_current_blog_id() ) -- there has been comments related to how this is an incomplete fix due to limitations of the preg_replace regex.

"pluggable.php" is similar to 25239.patch

"CVE-2017-8295.patch" uses parse_url() to extract the domain from network_home_url()

Recommendations that lack patches:

Creating a new wp_option value for the site's maintenance email address, which would be used in the From field for these sort of emails

---

There are still two open questions related to:

"Whose responsibility is it to police SERVER_NAME in a multi-domain configuration? "

"What are the arguments against having an option for setting the email address with a sensible default?"
Which have gone answered and I don't want to quash these questions with the summary.

So please, chime in if you feel a solution which patches may be a good fix ASAP (since security) and which are more complete fixes (or if you have another proposal please add it here).

---

IMHO I feel the wp_options value proposal is a more complete fix, but requires a more work and testing (and requires a fall-back in case this value of this option is NULL). While the CVE-2017-8295.patch fits the bill for a sufficient "just works" patch if someone wants to kabash that security issue and these bugs for the short term and give more time for a more complete fix to be tested, verified and implemented.

[BjornW]

@rawrly thank you for summarizing, I'd like to add that there is a solution which includes using a new setting for the From field our plugin does this. Have a look at it here: ​https://wordpress.org/plugins/admin-email-as-from-address/

[pessoft]

• If client supplied data ( like contents of $_SERVER['SERVER_NAME'] ) are not validated securely then it shouldn't be used. So replacement of all such occurrences with trusted data source makes sense. layotte's attachment:server_name.diff​ shows where these replacements should be done in preparation of From email addresses.
• For source of the domain I think that solution suggested in cloudstek's attachment:CVE-2017-8295.patch​: parse_url( network_home_url(), PHP_URL_HOST ) looks better.
• Also for backward compatibility I would also consider to keep the www. removal.
• Usage of always available domain name from network_home_url() instead unreliable $_SERVER['SERVER_NAME'] fixes also issues for those who call WP directly by PHP ( for example in situations where WP cron is invoked using system cron and php-cli ).

I'll put together an attachment where I combine the solutions as described in the points above, in a hope to speed up fix for this ticket.

[pessoft]

Use network_home_url() as a source of From email's domain instead of SERVER_NAME

[kitchin]

Contrary to comments above, general opinion is that while HTTP_HOST can be unsafe client data, SERVER_NAME is a server configuration and so pretty safe. For example, ​https://stackoverflow.com/questions/2297403/http-host-vs-server-name

That may not be 100% guaranteed on all servers, so distrusting SERVER_NAME may be wise, but comment:91 is not generally right about "client supplied data."

Also, grepping the trunk code base...

SERVER_NAME (excluding OPENSSL_TLSEXT_SERVER_NAME) is found in:
12 hits in 5 files

HTTP_HOST is found in:
26 hits in 14 files

Figures are the same for the current release, WP 4.8.

[RedSand]

Replying to kitchin:

Contrary to comments above, general opinion is that while HTTP_HOST can be unsafe client data, SERVER_NAME is a server configuration and so pretty safe. For example, ​https://stackoverflow.com/questions/2297403/http-host-vs-server-name

That may not be 100% guaranteed on all servers, so distrusting SERVER_NAME may be wise, but comment:91 is not generally right about "client supplied data."

I'd disagree on that. If UseCanonicalName (or equivalent) is not set properly on the server, then the SERVER_NAME can be overridden with the value of HTTP_HOST. Remember that "pretty safe" != "secure". I would say that comments by @pessoft were correct in the general meaning.

In general I agree with @pessoft's recommendation. As he noted, it also would take of of issues with CLI.

[seayou]

Having the same issue with my nginx configuration. I have default_server set in my configuration.

I was thinking about that if it's safe to set fastcgi_param SERVER_NAME "something.com"; in nginx as I deliberately don't want server_name to be set. Could that have any negative effect in other parts of WP or plugins (well that's a long shot)?

[SergeyBiryukov]

#42011 was marked as a duplicate.

[dvershinin]

Replying to seayou:

Having the same issue with my nginx configuration. I have default_server set in my configuration.

I was thinking about that if it's safe to set fastcgi_param SERVER_NAME "something.com"; in nginx as I deliberately don't want server_name to be set. Could that have any negative effect in other parts of WP or plugins (well that's a long shot)?

I can think that it's a good solution, but you might stay unprotected from possible (future?) vulnerabilities that lie with use of HTTP_HOST (as opposed to SERVER_NAME) in PHP code.

Maybe better of making sure that only specific Host header field values are accepted for default_server. Like ​this (it is implementation of canonical host names in nginx, sort of).

[pessoft]

Replying to kitchin and RedSand:

Thank you both for your feedback. Indeed, in my comment I tried to point out the general meaning for SERVER_NAME replacement, which could support the solution of this ticket. Also, I aimed for performing replacements related just to the ticket's topic.

Does anyone see any risks or disadvantages for suggested approach ( and eventually better alternatives )?

[pessoft]

Looking on this topic from a different perspective:

What is the officially supported and/or recommended value for Server Name in web server configurations, which serve WordPress instances?

[swissspidy]

#44569 was marked as a duplicate.

[SergeyBiryukov]

#43878 was marked as a duplicate.

[bastho]

Hi,
I just discover this ticket (I've opened a duplicate).

Here is my thought:

[38287] forces PHPMailer's setFrom() to don't set Sender field.

It seems to improve compatibility on some server environments, but it's a regression on those which support -f option. On these ones, the Sender is something like www-data@host which can cause SPF failure if the server name doesn't match the domain name (which occurs pretty often).

While this setting is very dependent of the server environment, I suggest to add a constant WP_MAIL_AUTO_SENDER to determine the expected behavior.

I know that introducing this constant does not match the "Decision, not option" directive, but I can't figure how to determine if the server supports or not the -f option.

This patch https://core.trac.wordpress.org/attachment/ticket/25239/25239.patch makes the trick.

[chesio]

Refresh of 23236.7.patch for current trunk (revision 44770)

[Starbuck]

This is not only related to email. In comment:92 @kitchin noted that there are 12 files referencing SERVER_NAME. One of the references is in v5.2.2 rest-api.php, which is throwing errors over nginx.

I'm not sure how I fell into that code segment yet. I'm not running SSL yet on this new site. But in diagnosing the issue I found this ticket.

[chesio]

Refresh of 23236.7.patch for current trunk (revision 47544)

[afragen]

​https://github.com/WordPress/wp-autoupdates/issues/89 was closed as a duplicate of this ticket.

[ocean90]

#40810 was marked as a duplicate.

[ocean90]

See also #40810 for some recent updates regarding this issue.

[Mte90]

Seems also #47733 it is a duplicate

[SergeyBiryukov]

#50532 was marked as a duplicate.

[SergeyBiryukov]

#50532 was marked as a duplicate.

[whyisjake]

In 48601:

Mail: Ensure that a server hostname can be set by using network_home_url().

Due to the varying server setups, $_SERVER['SERVER_NAME']; can't reliably ensure that there will be a relevant host that could be used as the hostname in an email. Since the network_home_url() is set at this point, and is filterable, this should better enable emails to be sent from the server.

Fixes #25239.
Props layotte, SergeyBiryukov, nacin, sreedoap, szepe.viktor, jesin, kitchin, mensmaximus, mt8.biz, Grzegorz.Janoszka, cbutlerjr, dd32, BjornW, neodjandre, BjornW, Ipstenu, ocean90, danielbachhuber, desmith, joemcgill, jdgrimes, riasat, derekakelly, swissspidy, lilmike, tsimmons, robert.peake, DavidAnderson, cloudstek, pigdog234, BjornW, rawrly, pessoft, RedSand, seayou, dvershinin, bastho, chesio, Starbuck, afragen, ocean90, whyisjake.