php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #74005 mail.add_x_header causes RFC-breaking lone line feed, loss of subsequent header
Submitted: 2017-01-26 23:11 UTC Modified: 2017-02-02 12:50 UTC
Votes:6
Avg. Score:4.7 ± 0.7
Reproduced:6 of 6 (100.0%)
Same Version:4 (66.7%)
Same OS:6 (100.0%)
From: andy_schmidt at HM-Software dot com Assigned: ab
Status: Closed Package: Mail related
PHP Version: 7.0.15 OS: Windows
Private report: No CVE-ID:
 [2017-01-26 23:11 UTC] andy_schmidt at HM-Software dot com
Description:
------------
Since version 7 the Windows implementation of mail() duplicates the "From:" header, e.g.:

Date: Thu, 26 Jan 2017 17:48:13 -0500
Subject: test subject
To: recipient@test.com
X-PHP-Originating-Script: 0:andytest.php
From: duplicated@test.com
From: duplicated@test.com

Under version 5 (e.g., 5.3.28) and prior there is only ONE "From:" header.

Gmail has now started to outright BLOCKING PHP generated emails due to "ambiguous" "from" information, therefore escalating this matter to one of highest priority. Here the SMTP status code from gmail when delivery is attempted by PHP 7 (works with PHP 5):

550-5.7.1 [74.201.226.50      11] Our system has detected that this message is
550-5.7.1 not RFC 5322 compliant:
550-5.7.1 Multiple 'From' headers found.
550-5.7.1 To reduce the amount of spam sent to Gmail, this message has been
550-5.7.1 blocked. Please visit
550-5.7.1  https://support.google.com/mail/?p=RfcMessageNonCompliant
550 5.7.1 and review RFC 5322 specifications for more information. d4si1478127qtf.33 - gsmtp






Test script:
---------------
<?php
$result = mail( "recipient@test.com", "test subject", "test content", "From: duplicated@test.com\r\n" );
?>


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-01-30 08:30 UTC] cafbca62 at opayq dot com
I confirm the problem. My Wordpress on IIS after update to PHP 7 has started creating this trouble.
 [2017-01-30 13:43 UTC] ab@php.net
-Status: Open +Status: Feedback
 [2017-01-30 13:43 UTC] ab@php.net
Thanks for the report. Please post also the related ini config.

Thanks.
 [2017-01-30 14:10 UTC] andy_schmidt at HM-Software dot com
-Status: Feedback +Status: Open
 [2017-01-30 14:10 UTC] andy_schmidt at HM-Software dot com
I tried to post the PHP.INI content, but your web site responds with:

"ERROR: Your comment looks like SPAM by its content. Please consider rewording."

and I see no "upload/file attach" button on your form.

On to plan "C"... The following download link will be valid for ONE week:
https://mediacenter.email/download/2072a12607534fca90f2a065223/9f5bcbd3
 [2017-01-31 00:43 UTC] ab@php.net
-Status: Open +Status: Feedback
 [2017-01-31 00:43 UTC] ab@php.net
I was asking for only the relevant ini, that would pass into a post, but thanks for sharing anyway. Unfortunately i still don't reproduce the behavior,the only diff option in your ini is mail.add_x_header, as far as i see. In PHP 5 the code flow is same. Still PHP will send also MAIL FROM command, maybe that could cause some misbehavior with the MTA, as the header itself is sent, too. But also RCPT TO command is sent, so just to wonder why it wouldn't cause a duplicated to header.

I still have to ask for more info on how to reproduce this behavior. Where do you get the header list from?

Thanks.
 [2017-01-31 15:14 UTC] andy_schmidt at HM-Software dot com
-Status: Feedback +Status: Open
 [2017-01-31 15:14 UTC] andy_schmidt at HM-Software dot com
Hi - can you please confirm that you ARE testing this on a Windows system? The implementation of mail() in Windows differs from the Linux implementation in SEVERAL aspects! For one, Windows uses direct SMTP delivery instead of a a "mailer" (such as "sendmail").

I ran another test that might help narrow down at what point the extra From: header is generated.

 $result = mail( "parmto@domain.com", "test subject", "test content", "From: PFHeader <hdrfrom@domain.com>\r\nTo: PTHeader <hdrto@domain.com>\r\n\Subject: hdrsubject\r\n" );

produces this:

Date: Tue, 31 Jan 2017 09:15:35 -0500
Subject: test subject
X-PHP-Originating-Script: 0:andytest.php
From: PFHeader <hdrfrom@domain.com>
To: PTHeader <hdrto@domain.com>
Subject: hdrsubject
From: hdrfrom@domain.com

It shows that the Date, Subject and X-PHP headers come first, so they seem to be created during the time when the parameter list and the config settings are processed.

You can also see that the next step is that the supplied headers are "appended as is", including any (unnecessary) "Subject" header and the bracketed "From" and "To" headers that each have the "name" component.

THREE things are important to note from this:

1. My original minimal test had supplied no "To:" header, so one had been created between the "Subject" and "X-PHP..." header. It's clear from this second test, that the generating of the "To:" header is done conditionally, after checking if such a header already exists. (The same sanity check is NOT done for the Subject header, FWIW)

2. The troublesome second "From" header is generated AFTER the headers from the parameters had been appended.

If I look at this from a programming perspective, I would theorize that there are four distinct phases in the code:

a) Parameter are processed. The "to" information is stored away so it can be used later for the RCPT TO commands. 

b) Date and Subject headers are generated unconditionally, a "To" header is generated if none is in the "headers" parameter, the X-PHP header is generated based on config settings.

c) The "headers" parameter is appended AS IS.

d) The SMTP commands are prepared. The comma-separated list of "to" parameters are turned into RCPT TO commands. Either already during the first phase or only now the "headers" parameter list is parsed for "From" information. Only the unbracketed email address portion is extracted. The MAIL FROM command is generated - and a matching (duplicate) "From:" header is appended to the headers.

Since the preparation/sending of SMTP commands via TCP/IP is unique to the Windows implementation, you will not reproduce that elsewhere!


This happens to be my area of expertise. Allow me to add some detail to avoid any miscommunication. Feel free to skip over this if the SMTP protocol and RFCs are your home-turf as well.
"HELO/EHLO", "MAIL FROM", "RCPT TO", "DATA" are not "generated" in the same sense that the headers are "generated", but they are the actual commands being issued via TCP/IP. These commands ONLY affect how the email is routed to the final recipient. The headers do NOT. On the other hand, these commands do not travel with/as part of the email. Instead each "hop" along the way, will issue its own/new set/subset of commands as it relays the message data to the next agent. (Per example 5 "RCPT TO" commands originally sent by PHP, will eventually result 5 copies of the DATA being sent  with 1 RCPT TO command each to 5 different servers handling 5 different recipient domains!)

On the other hand, the "To:", "From:", "CC:" headers are completely embedded inside the data - and are entirely irrelevant to the sending/relaying process and often aren't even factual (e.g., don't match the MAIL FROM, RCTP TO literally). They are just "metadata" that the recipient's email client will use to display the "apparent" sender/recipient, subject, data, etc.

So, it's not only normal but absolutely necessary, that you will have MAIL FROM and RCPT TO commands taking care of delivery, and ALSO to have To/From headers supplying metadata to the email client software of the recipient.

Only AFTER the emails have reached the final destination, will a server (like Gmails) peek inside the email data to see if the content looks legitimate. Besides scanning the email body and attachments, the headers are helpful in this process - because imperfect/deceptive/mismatching headers hint at a possibly illegitimate message. Which is what's happening here.

I have observed the headers generated by PHP by looking at the DATA message stream that the mail server spools to disk, so that it can be relayed to the next/destination server. When the test script is executed by PHP 5, it outputs DATA that has only the ONE "From:" from the mail() parameter list, if the test script is executed by PHP 7, it has a second "From:".

I did notice in the change log that the mail function (specially handling of headers) has seen some significant work - so I'm not surprised that something could have been introduced accidentally.
 [2017-01-31 15:54 UTC] andy_schmidt at HM-Software dot com
PS - rereading your note "PHP will send also MAIL FROM command, maybe that could cause some misbehavior with the MTA, as the header itself is sent, too. But also RCPT TO command is sent, so just to wonder why it wouldn't cause a duplicated to header", you seem to imply a relationship between headers and SMTP conversation that doesn't exist.

Here a VERY common scenario. An email from "a" to "b" with a CC to "x". The headers for that email will say:

From: a@sender.com
To: b@recipient.com
Cc: x@recipient.com

With these headers (metadata), the two recipient will open the email in their clients and be presented the exact same from/to/cc information.

However, the email is actually delivered through a mailing service or contact management system that uses bounces to clean up contact lists. When the email is delivered to "x", the following command sequence will be used:

HELO
MAIL FROM: bounce@sender.com
RCPT TO: x@recipient.com
DATA
[followed by headers and email body]
.
QUIT

As you can easily see, in this case the "From" and "To" header information does NOT appear in the SMTP commands AT ALL.

The historic mail() function complicates matters because its parameter list was probably conceived before there was web hosting, e.g., at a time when "from" would be the system default, or could be automatically determined when the "sendmail" is launched in the user context. As a result, today we have a parameter list that is missing the crucial "From", and the mail function has to scramble that information from elsewhere. On the other hand, we have a "subject" parameter that clashes with a full set of other headers supplied as the next parameter.  I'm not complaining - just trying to explain why things may seem a bit out of synch.
 [2017-01-31 17:45 UTC] ab@php.net
-Status: Open +Status: Feedback
 [2017-01-31 17:45 UTC] ab@php.net
Thanks for the further explanation, @andy_schmidt. Yes, i'm checking on Windows, and yes - i'm doing literally what you write to reproduce. I was asking you just where do you get the header list you post :) Now, I've put a primitive test facility, to see what is sent. Here you are https://github.com/php/php-src/commit/163bb87897c82eb7067e2a2e89818293a455e7a3

POST: 'HELO
'
POST: 'MAIL FROM:<duplicated@test.com>
'
POST: 'RCPT TO:<recipient@test.com>
'
POST: 'DATA
'
POST: 'Date: Tue, 31 Jan 2017 12:39:21 -0500
Subject: test subject
To: recipient@test.com
X-PHP-Originating-Script: 0:bug74005.php
From: duplicated@test.com
'
POST: '
'
POST: 'test content'
POST: '
.
'
POST: 'QUIT
'

The PHP code is from your first post. Unfortunately, the way you write to be a reproducer doesn't work on my side. I see only one from header. Maybe it needs some real MTA or something else, that is not contained in your description. As long as the wrong behavior can't be reproduced and debugged, there's a little chance to understand, where the bug is, and to actually fix it. Please help me with that.

Thanks.
 [2017-01-31 18:45 UTC] andy_schmidt at HM-Software dot com
-Summary: mail function duplicates FROM header +Summary: mail.add_x_header causes RFC-breaking lone line feed, loss of subsequent header -Status: Feedback +Status: Open
 [2017-01-31 18:45 UTC] andy_schmidt at HM-Software dot com
Thanks for passing this on - THAT looks like a feasible test. 

Seeing that THAT didn't reproduce it made me take an extra step and it turns out the "duplicate From header" is actually a secondary problem, which is why you don't see it.

I have identified the ACTUAL trigger by looking at the output in HEX. The real bug is with the mail.add_x_header config option in 7.0. Are you testing with 7.0? Then try outputting the headers (after DATA was sent) in HEX. In MY test 7.0 incorrectly ends the X-PHP header like THIS:

  X-PHP-Originating-Script: 0:andytest.php\n

while 5.x correctly ends like THIS:

  X-PHP-Originating-Script: 0:andytest.php\r\n

The problem is (once again) the infamous "lone line feed" which is NEVER permitted by SMTP.

The secondary behavior is triggers is, that the default Windows MTA doesn't see a proper line-end at the end of the X-PHP header; the subsequent "From" header is therefor seen as being part of that same line.

Because the output from PHP is lacking a recognizable, required "From" header, a default header is added by the MTA (not PHP, as previously assumed). Other (Linux based) mail systems DO treat a lone "LF" as a legitimate line end - and thus may choke on the duplicate From header they DO see.

I suspect that you will be able to reproduce the original "lone linefeed" problem with 7.0, if your test outputs any \n and \r occurrences in HEX. (The duplicate "From" header is actually just a secondary problem.)

I have confirmed that
mail.add_x_header = Off
will circumvent this bug under 7.0.
 [2017-01-31 23:54 UTC] andy_schmidt at HM-Software dot com
I think I have it worked out. The X-PHP... header likely had ALWAYS ended with a lone LF.

However, with PHP 5 (and prior) either the mail function itself, or the win32 sendmail.c performed a "fixup" where it replaced all lone LFs with valid CRLFs to arrive at RFC compliant formatting. That made good sense, because Linux's own line-end is just LF, so the Linux mailers actually always expected single LFs, and had always outputted CRLF to the MTA.

With PHP 7.0 that "fixup" is no longer performed with win32.  The result is that your "own" malformed X-PHP... header, terminated by a lone LF, is now bleeding through to the MTA "unfixed".

I have confirmed this behavior by testing THIS script under PHP 5 and PHP 7 - intentionally ending each additional header with a lone LF:

$result = mail( "recipient@domain.com", "test subject", "test content", "X-PHP: ".phpversion()."\nFrom: sender@domain.com\n" );

Notice how I intentionally use malformed additional headers with just lone LFs!

Result with PHP 5: 

Date: Tue, 31 Jan 2017 18:16:07 -0500\r\n
Subject: test subject\r\n
To: recipient@domain.com\r\n
X-PHP-Originating-Script: 0:andytest.php\r\n
X-PHP: 5.3.28\r\n
From: sender@domain.com\r\n

Result with PHP 7:

Date: Tue, 31 Jan 2017 18:14:29 -0500\r\n
Subject: test subject\r\n
To: recipient@domain.com\r\n
X-PHP-Originating-Script: 0:andytest.php\n
X-PHP: 7.0.15\n
From: sender@domain.com\r\n

As this shows, with PHP 5 each individual occurrence of a lone LF in the headers is globally replaced with a proper CRLF (including your own X-PHP... header).

With PHP 7, the various lone LF embedded in the additional headers are NO LONGER "fixed", only the FINAL LF (terminating the additional headers) is fixed to a CRLF.

So this didn't break because a change to the X-PHP... header - it had always been "wrong". It broke because of a v7 change to mail() or win32 sendmail where it no longer corrects bad formatting.


The greater impact of that is with WordPress and other PHP-based CMS systems under Windows. They will work fine under Linux, because there the lone LF are handled by the mailer. They will work fine under Windows with PHP 5, because it fixes the lone LFs to proper CRLF.

However, if a Windows server is upgraded to PHP 7, THEN the various CMS will fail email delivery, because now the lone LFs are passed through to the MTA.

For your information: WordPress (and others) use PHPmailer 5, and PHPmailer 5 uses mail().
 [2017-02-01 09:15 UTC] yohgaki@php.net
Thank you for spotting what's wrong.
It's easy bug to be fixed. 

ext/standard/mail.c
	if (PG(mail_x_header)) {
		const char *tmp = zend_get_executed_filename();
		zend_string *f;

		f = php_basename(tmp, strlen(tmp), NULL, 0);

		if (headers != NULL && *headers) {
			spprintf(&hdr, 0, "X-PHP-Originating-Script: " ZEND_LONG_FMT ":%s\n%s", php_getuid(), ZSTR_VAL(f), headers);
		} else {
			spprintf(&hdr, 0, "X-PHP-Originating-Script: " ZEND_LONG_FMT ":%s", php_getuid(), ZSTR_VAL(f));
		}
		zend_string_release(f);
	}

All versions should use "\r\n" rather than "\n", including 5.6.

BTW, I don't like line ending conversions. Suppose user script validate invalid "\r\n" in mail header string, e.g. mail address, but forgot to check "\r" and/or "\n", then line ending conversion by PHP would allow mail header injection. Recent mail() is made to reject multiple or broken line ending in extra headers, but it still allows "\n" for compatibility sake. This is irrelevant to this bug, just for the record. 

@anatol Since I'm not sure if this bug fix can be applied to 5.6, could you apply the fix? 

Mixing line ending chars is mess. It seems we are better to deprecate string extra headers and force users to use array extra headers someday.
 [2017-02-01 11:59 UTC] ab@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=ec43a11581f457bd252d98e948d7a0531b4fdfc2
Log: Fixed bug #74005 mail.add_x_header causes RFC-breaking lone line feed
 [2017-02-01 11:59 UTC] ab@php.net
-Status: Open +Status: Closed
 [2017-02-01 12:08 UTC] ab@php.net
-Assigned To: +Assigned To: ab
 [2017-02-01 12:08 UTC] ab@php.net
Yeah, that's the exact place, Yasuo. Clearly it's not RFC 5322 complaint, but not that easy. From the git history, see also bug #48620 and bug #50907 where it was already made to use CRLF and then reverted back to LF. Well, welcome to the real world :) We deal with a very legacy code here, so probably should be reluctant to to change this in the hurry. But also, i've found out, that there was a porting mistake, so then the headers in 7.0 was not noramlized anymore in win32/sendmail.c - pushed a fix touching that direction in first place.

@andy_schmidt, thanks for the further investigation. Please check the snap from ec43a11581f457bd252d98e948d7a0531b4fdfc2 or any later. Also if anyone of the voters in this ticket could check as well, it'd be great. We miss the tests for this part badly, probably some could be written using a socket server as a dummy MTA. If someone has time to do taht right now, please file a PR!

Thanks.
 [2017-02-01 15:04 UTC] andy_schmidt at HM-Software dot com
I probably wouldn't backport this to PHP 5, because in PHP 5 you were still fixing lone line feeds in the win32 implementation. So the bad X-PHP... header had no ill effect.

Under PHP 7 there is need to action because now the headers are "passed through" under win32 - so they HAVE to be SMTP compliant.

I have reviewed the earlier patch you cited:
https://bugs.php.net/bug.php?id=48620

It shows the original code to have been:
spprintf(&hdr, 0, "%s\r\nX-PHP-Originating-Script: %ld:%s\n", headers, php_getuid(), f);
so it unconditionally bracketed the X-PHP header with a leading CRLF but trailing lone LF!? This was REALLY bad code - because it was following NEITHER convention. With an old Linux mailer, the leading CRLF would be treated as two line feeds. It also caused the other reported problem if there were no user supplied headers in which case the (sole) X-PHP header would start with the CRLF (essentially creating an empty header)!

The attempted fix only sought to address the second scenario by NOT starting with a CRLF if a previous header existed. It still kept the inconsistency of a leading CRLF but a trailing lone LF.


Your "legacy" challenge is reconciling two directly opposing requirements:

- mail() under Linux was talking to a mailer, some of which had required the input to be operating system line end (single line feed) and rejected/mangled any SMTP compliant CR/LF.
- mail() under win32 on the other hand is talking directly to SMTP, which absolutely mandates RFC compliant line-ends.

Before PHP 7 you had taken the logical/pragmatic step of simply adapting the win32 implementation to fix any lone LF (or CR or LFCR?) to CRLF (which is a simple RegEx and kept everyone happy).

If you no longer want to do that for PHP 7, then you would need to reject non-RFC compliant additionalheaders (including your own broken X-PHP... header!). But, I suspect you'd effect a lot of existing applications that relied on single line feeds. Alternatively you would have to reinstate the PHP 5 behavior for win 32 thus once again aligning the behavior for both operating system families until you are ready to complete change the additionalheaders from "string" to "array".
 [2017-02-01 18:24 UTC] ab@php.net
Yeah, that's the exact point why it was done with CRLF and then reverted. The porting mistake in 7.x was fixed in the aforementioned commit, thus the status quo with 5.x should be now restored. Please fetch the latest http://windows.php.net/snapshots/ to ensure. Otherwise, the legacy code should not be touched in the stable branch, IMO. Furthermore, nowadays where things like Horde and other exist, significantly changing the legacy functions probably is not very big priority.

Thanks.
 [2017-02-01 22:40 UTC] andy_schmidt at HM-Software dot com
Tested successfully with snapshot using the minimal test, even with mail.add_x_header ON (and also confirmed with virgin WordPress):

$result = mail( "recipient@domain.com", "test subject", "test content", "X-PHP: ".phpversion()."\nFrom: sender@domain.com\n" );

Result now matches result from PHP 5:

Date: Wed, 01 Feb 2017 17:19:53 -0500\r\n
Subject: test subject\r\n
To: recipient@domain.com\r\n
X-PHP-Originating-Script: 0:andytest.php\r\n
X-PHP: 7.0.17-dev\r\n
From: sender@domain.com\r\n

Thank you for taking care of that. I trust this will become part of 7.1 as well.
 [2017-02-02 00:14 UTC] ab@php.net
Many thanks for checking the snaps and actually investigating the cause of the issue :) The headers passed are now normalized internally same way they are in 5.x And of course, we merge fixes through all the branches, so it'll be in all the stable 7.x and later. Unfortunately too late for the upcoming finals, but those after them will get it.

Thanks.
 [2017-02-02 01:46 UTC] yohgaki@php.net
I should have noticed when I modify mail.c

ext/standard/mail.c
	if (PG(mail_x_header)) {
		const char *tmp = zend_get_executed_filename();
		zend_string *f;

		f = php_basename(tmp, strlen(tmp), NULL, 0);

		if (headers != NULL && *headers) {
			spprintf(&hdr, 0, "X-PHP-Originating-Script: " ZEND_LONG_FMT ":%s\n%s", php_getuid(), ZSTR_VAL(f), headers);
		} else {
			spprintf(&hdr, 0, "X-PHP-Originating-Script: " ZEND_LONG_FMT ":%s", php_getuid(), ZSTR_VAL(f));
		}
		zend_string_release(f);
	}

BTW, I don't like line ending conversions. Suppose user script validate invalid "\r\n" in mail header string, e.g. mail address, but forgot to check "\r" and/or "\n", then line ending conversion by PHP would allow mail header injection. Recent mail() is made to reject multiple or broken line ending in extra headers, but it still allows "\n" for compatibility purpose. 

Mixing line ending chars is mess. It seems we are better to deprecate string extra headers and force users to use array extra headers someday. And/Or disallow "\n" in header. We may better to disallow "\n" in 7.2.
 [2017-02-02 08:21 UTC] yohgaki@php.net
Sorry, I thought my [2017-02-01 09:15 UTC] post was't went though.

Anyway, line ending is mess... Some sendmail accepts "\n".
We may better to have INI setting that normalize line ending chars. We shouldn't convert "\n","\r" to "\r\n", but we can convert "\r\n" to "\n" safely. 

mail.convert_crlf_to_lf = On/Off.

Then accept only CR/LF line ending. This would resolve issues on almost all mail systems. It breaks existing code, though.
 [2017-02-02 12:50 UTC] ab@php.net
For now, it's just the same behavior in both 5 and 7. This way we simply keep BC, and further improvements can base on this. But, as for me, there's no reason we should invest more into this. In any form currently, it is not RFC complaint, and the suggestion doesn't makes it such. Reading the header related sections https://www.heise.de/netze/rfc/rfcs/rfc5322.shtml#page-8 - either way the folding/unfolding is not and won't be supported by mail(). Either it enforces CRLF, or allows lone LF. IMHO, the PHP frameworks available are far better choice, as it would need much more to support the full RFC implementation). But also, the real world differences in MTAs are the big factor, as is to see from the previous issues.

With the security concern - yes, a negligent programming could cause some issue. According to our current security classification - it is not a security issue. Otherwise in general - some program that accepts headers from the userspace is rather a very improbable or very special case, that needs an extra care anyway. For just accepting an email and to compose the header string the precaution is rather trivial.

Thanks.
 [2017-03-04 16:08 UTC] quentin dot lengele at gmail dot com
Hi guys, 

I have similar problems on PhP 7.1.2 (nts x64 for windows).

550-5.7.1 not RFC 5322 compliant:
550-5.7.1 Multiple 'From' headers found.


My server is Windows 2008R2 IIS 7.5, running PhP with FastCGI

All was running OK with PhP 5.x

Is this really fixed?
 [2017-03-04 16:28 UTC] quentin dot lengele at gmail dot com
My bad, it seems the bug is somewhere else. Took me days to figures out. I'm sorry.
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Wed Jun 28 19:01:44 2017 UTC