diff options
author | Christoph M. Becker <cmbecker69@gmx.de> | 2021-03-01 16:18:40 +0100 |
---|---|---|
committer | Christoph M. Becker <cmbecker69@gmx.de> | 2021-03-01 18:46:21 +0100 |
commit | 71297a254b8f0d97c028f3324cbaf95adf8de33c (patch) | |
tree | d79731f1d753a65ecafdf13cf29dbbd49d871706 | |
parent | 2c508c4d407e98a27ed2631ae88e2e10ee430003 (diff) | |
download | php-git-71297a254b8f0d97c028f3324cbaf95adf8de33c.tar.gz |
Fix #80751: Comma in recipient name breaks email delivery
So far, `SendText()` simply separates potential email address lists at
any comma, disregarding that commas inside a quoted-string do not
delimit addresses. We fix that by introducing an own variant of
`strtok_r()` which caters to quoted-strings.
We also make `FormatEmailAddress()` aware of quoted strings.
We do not cater to email address comments, and potentially other quirks
of RFC 5322 email addresses, but catering to quoted-strings is supposed
to solve almost all practical use cases.
Co-authored-by: Nikita Popov <nikita.ppv@gmail.com>
Closes GH-6735.
-rw-r--r-- | ext/standard/tests/mail/bug80751.phpt | 93 | ||||
-rw-r--r-- | win32/sendmail.c | 102 |
2 files changed, 179 insertions, 16 deletions
diff --git a/ext/standard/tests/mail/bug80751.phpt b/ext/standard/tests/mail/bug80751.phpt new file mode 100644 index 0000000000..d90be83851 --- /dev/null +++ b/ext/standard/tests/mail/bug80751.phpt @@ -0,0 +1,93 @@ +--TEST-- +Bug #80751 (Comma in recipient name breaks email delivery) +--SKIPIF-- +<?php +if (PHP_OS_FAMILY !== 'Windows') die('skip Windows only test'); +if (getenv("SKIP_SLOW_TESTS")) die('skip slow test'); +require_once __DIR__ . '/mail_skipif.inc'; +?> +--INI-- +SMTP=localhost +smtp_port=25 +--FILE-- +<?php +require_once __DIR__ . '/mail_include.inc'; + +function find_and_delete_message($username, $subject) { + global $default_mailbox, $password; + + $imap_stream = imap_open($default_mailbox, $username, $password); + if ($imap_stream === false) { + die("Cannot connect to IMAP server $server: " . imap_last_error() . "\n"); + } + + $found = false; + $repeat_count = 20; // we will repeat a max of 20 times + while (!$found && $repeat_count > 0) { + // sleep for a while to allow msg to be delivered + sleep(1); + + $num_messages = imap_check($imap_stream)->Nmsgs; + for ($i = $num_messages; $i > 0; $i--) { + $info = imap_headerinfo($imap_stream, $i); + if ($info->subject === $subject) { + $header = imap_fetchheader($imap_stream, $i); + echo "Return-Path header found: "; + var_dump(strpos($header, 'Return-Path: joe@example.com') !== false); + echo "To header found: "; + var_dump(strpos($header, 'To: "<bob@example.com>" <info@mail.local>') !== false); + echo "From header found: "; + var_dump(strpos($header, 'From: "<bob@example.com>" <joe@example.com>') !== false); + echo "Cc header found: "; + var_dump(strpos($header, 'Cc: "Lastname, Firstname\\\\" <admin@mail.local>') !== false); + imap_delete($imap_stream, $i); + $found = true; + break; + } + } + $repeat_count--; + } + + imap_close($imap_stream, CL_EXPUNGE); + return $found; +} + +$to = "\"<bob@example.com>\" <{$users[1]}@$domain>"; +$subject = bin2hex(random_bytes(16)); +$message = 'hello'; +$headers = "From: \"<bob@example.com>\" <joe@example.com>\r\n" + . "Cc: \"Lastname, Firstname\\\\\" <{$users[2]}@$domain>\r\n" + . "Bcc: \"Firstname \\\"Ni,ck\\\" Lastname\" <{$users[3]}@$domain>\r\n"; + +$res = mail($to, $subject, $message, $headers); +if ($res !== true) { + die("TEST FAILED : Unable to send test email\n"); +} else { + echo "Message sent OK\n"; +} + +foreach ([$users[1], $users[2], $users[3]] as $user) { + if (!find_and_delete_message("$user@$domain", $subject)) { + echo "TEST FAILED: email not delivered\n"; + } else { + echo "TEST PASSED: Message sent and deleted OK\n"; + } +} +?> +--EXPECT-- +Message sent OK +Return-Path header found: bool(true) +To header found: bool(true) +From header found: bool(true) +Cc header found: bool(true) +TEST PASSED: Message sent and deleted OK +Return-Path header found: bool(true) +To header found: bool(true) +From header found: bool(true) +Cc header found: bool(true) +TEST PASSED: Message sent and deleted OK +Return-Path header found: bool(true) +To header found: bool(true) +From header found: bool(true) +Cc header found: bool(true) +TEST PASSED: Message sent and deleted OK diff --git a/win32/sendmail.c b/win32/sendmail.c index 78409b53a8..ee017374f4 100644 --- a/win32/sendmail.c +++ b/win32/sendmail.c @@ -333,6 +333,37 @@ PHPAPI char *GetSMErrorText(int index) } } +/* strtok_r like, but recognizes quoted-strings */ +static char *find_address(char *list, char **state) +{ + zend_bool in_quotes = 0; + char *p = list; + + if (list == NULL) { + if (*state == NULL) { + return NULL; + } + p = list = *state; + } + *state = NULL; + while ((p = strpbrk(p, ",\"\\")) != NULL) { + if (*p == '\\' && in_quotes) { + if (p[1] == '\0') { + /* invalid address; let SMTP server deal with it */ + break; + } + p++; + } else if (*p == '"') { + in_quotes = !in_quotes; + } else if (*p == ',' && !in_quotes) { + *p = '\0'; + *state = p + 1; + break; + } + p++; + } + return list; +} /********************************************************************* // Name: SendText @@ -357,7 +388,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char { int res; char *p; - char *tempMailTo, *token, *pos1, *pos2; + char *tempMailTo, *token, *token_state, *pos1, *pos2; char *server_response = NULL; char *stripped_header = NULL; zend_string *data_cln; @@ -410,7 +441,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char tempMailTo = estrdup(mailTo); /* Send mail to all rcpt's */ - token = strtok(tempMailTo, ","); + token = find_address(tempMailTo, &token_state); while (token != NULL) { SMTP_SKIP_SPACE(token); @@ -424,14 +455,14 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char efree(tempMailTo); return (res); } - token = strtok(NULL, ","); + token = find_address(NULL, &token_state); } efree(tempMailTo); if (mailCc && *mailCc) { tempMailTo = estrdup(mailCc); /* Send mail to all rcpt's */ - token = strtok(tempMailTo, ","); + token = find_address(tempMailTo, &token_state); while (token != NULL) { SMTP_SKIP_SPACE(token); @@ -445,7 +476,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char efree(tempMailTo); return (res); } - token = strtok(NULL, ","); + token = find_address(NULL, &token_state); } efree(tempMailTo); } @@ -471,7 +502,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char tempMailTo = estrndup(pos1, pos2 - pos1); } - token = strtok(tempMailTo, ","); + token = find_address(tempMailTo, &token_state); while (token != NULL) { SMTP_SKIP_SPACE(token); @@ -485,7 +516,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char efree(tempMailTo); return (res); } - token = strtok(NULL, ","); + token = find_address(NULL,&token_state); } efree(tempMailTo); } @@ -496,7 +527,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char if (mailBcc && *mailBcc) { tempMailTo = estrdup(mailBcc); /* Send mail to all rcpt's */ - token = strtok(tempMailTo, ","); + token = find_address(tempMailTo, &token_state); while (token != NULL) { SMTP_SKIP_SPACE(token); @@ -510,7 +541,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char efree(tempMailTo); return (res); } - token = strtok(NULL, ","); + token = find_address(NULL, &token_state); } efree(tempMailTo); } @@ -544,7 +575,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char } } - token = strtok(tempMailTo, ","); + token = find_address(tempMailTo, &token_state); while (token != NULL) { SMTP_SKIP_SPACE(token); @@ -558,7 +589,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char efree(tempMailTo); return (res); } - token = strtok(NULL, ","); + token = find_address(NULL, &token_state); } efree(tempMailTo); @@ -978,6 +1009,46 @@ static unsigned long GetAddr(LPSTR szHost) return (lAddr); } /* end GetAddr() */ +/* returns the contents of an angle-addr (caller needs to efree) or NULL */ +static char *get_angle_addr(char *address) +{ + zend_bool in_quotes = 0; + char *p1 = address, *p2; + + while ((p1 = strpbrk(p1, "<\"\\")) != NULL) { + if (*p1 == '\\' && in_quotes) { + if (p1[1] == '\0') { + /* invalid address; let SMTP server deal with it */ + return NULL; + } + p1++; + } else if (*p1 == '"') { + in_quotes = !in_quotes; + } else if (*p1 == '<' && !in_quotes) { + break; + } + p1++; + } + if (p1 == NULL) return NULL; + p2 = ++p1; + while ((p2 = strpbrk(p2, ">\"\\")) != NULL) { + if (*p2 == '\\' && in_quotes) { + if (p2[1] == '\0') { + /* invalid address; let SMTP server deal with it */ + return NULL; + } + p2++; + } else if (*p2 == '"') { + in_quotes = !in_quotes; + } else if (*p2 == '>' && !in_quotes) { + break; + } + p2++; + } + if (p2 == NULL) return NULL; + + return estrndup(p1, p2 - p1); +} /********************************************************************* // Name: int FormatEmailAddress @@ -993,13 +1064,12 @@ static unsigned long GetAddr(LPSTR szHost) // History: //********************************************************************/ static int FormatEmailAddress(char* Buf, char* EmailAddress, char* FormatString) { - char *tmpAddress1, *tmpAddress2; + char *tmpAddress; int result; - if( (tmpAddress1 = strchr(EmailAddress, '<')) && (tmpAddress2 = strchr(tmpAddress1, '>')) ) { - *tmpAddress2 = 0; // terminate the string temporarily. - result = snprintf(Buf, MAIL_BUFFER_SIZE, FormatString , tmpAddress1+1); - *tmpAddress2 = '>'; // put it back the way it was. + if ((tmpAddress = get_angle_addr(EmailAddress)) != NULL) { + result = snprintf(Buf, MAIL_BUFFER_SIZE, FormatString, tmpAddress); + efree(tmpAddress); return result; } return snprintf(Buf, MAIL_BUFFER_SIZE , FormatString , EmailAddress ); |