Skip to content

Commit 71297a2

Browse files
cmb69cmb69
committed
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.
1 parent 2c508c4 commit 71297a2

File tree

2 files changed

+179
-16
lines changed

2 files changed

+179
-16
lines changed

‎ext/standard/tests/mail/bug80751.phpt

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
--TEST--
2+
Bug #80751 (Comma in recipient name breaks email delivery)
3+
--SKIPIF--
4+
<?php
5+
if (PHP_OS_FAMILY !== 'Windows') die('skip Windows only test');
6+
if (getenv("SKIP_SLOW_TESTS")) die('skip slow test');
7+
require_once __DIR__ . '/mail_skipif.inc';
8+
?>
9+
--INI--
10+
SMTP=localhost
11+
smtp_port=25
12+
--FILE--
13+
<?php
14+
require_once __DIR__ . '/mail_include.inc';
15+
16+
function find_and_delete_message($username, $subject) {
17+
global $default_mailbox, $password;
18+
19+
$imap_stream = imap_open($default_mailbox, $username, $password);
20+
if ($imap_stream === false) {
21+
die("Cannot connect to IMAP server $server: " . imap_last_error() . "\n");
22+
}
23+
24+
$found = false;
25+
$repeat_count = 20; // we will repeat a max of 20 times
26+
while (!$found && $repeat_count > 0) {
27+
// sleep for a while to allow msg to be delivered
28+
sleep(1);
29+
30+
$num_messages = imap_check($imap_stream)->Nmsgs;
31+
for ($i = $num_messages; $i > 0; $i--) {
32+
$info = imap_headerinfo($imap_stream, $i);
33+
if ($info->subject === $subject) {
34+
$header = imap_fetchheader($imap_stream, $i);
35+
echo "Return-Path header found: ";
36+
var_dump(strpos($header, 'Return-Path: joe@example.com') !== false);
37+
echo "To header found: ";
38+
var_dump(strpos($header, 'To: "<bob@example.com>" <info@mail.local>') !== false);
39+
echo "From header found: ";
40+
var_dump(strpos($header, 'From: "<bob@example.com>" <joe@example.com>') !== false);
41+
echo "Cc header found: ";
42+
var_dump(strpos($header, 'Cc: "Lastname, Firstname\\\\" <admin@mail.local>') !== false);
43+
imap_delete($imap_stream, $i);
44+
$found = true;
45+
break;
46+
}
47+
}
48+
$repeat_count--;
49+
}
50+
51+
imap_close($imap_stream, CL_EXPUNGE);
52+
return $found;
53+
}
54+
55+
$to = "\"<bob@example.com>\" <{$users[1]}@$domain>";
56+
$subject = bin2hex(random_bytes(16));
57+
$message = 'hello';
58+
$headers = "From: \"<bob@example.com>\" <joe@example.com>\r\n"
59+
. "Cc: \"Lastname, Firstname\\\\\" <{$users[2]}@$domain>\r\n"
60+
. "Bcc: \"Firstname \\\"Ni,ck\\\" Lastname\" <{$users[3]}@$domain>\r\n";
61+
62+
$res = mail($to, $subject, $message, $headers);
63+
if ($res !== true) {
64+
die("TEST FAILED : Unable to send test email\n");
65+
} else {
66+
echo "Message sent OK\n";
67+
}
68+
69+
foreach ([$users[1], $users[2], $users[3]] as $user) {
70+
if (!find_and_delete_message("$user@$domain", $subject)) {
71+
echo "TEST FAILED: email not delivered\n";
72+
} else {
73+
echo "TEST PASSED: Message sent and deleted OK\n";
74+
}
75+
}
76+
?>
77+
--EXPECT--
78+
Message sent OK
79+
Return-Path header found: bool(true)
80+
To header found: bool(true)
81+
From header found: bool(true)
82+
Cc header found: bool(true)
83+
TEST PASSED: Message sent and deleted OK
84+
Return-Path header found: bool(true)
85+
To header found: bool(true)
86+
From header found: bool(true)
87+
Cc header found: bool(true)
88+
TEST PASSED: Message sent and deleted OK
89+
Return-Path header found: bool(true)
90+
To header found: bool(true)
91+
From header found: bool(true)
92+
Cc header found: bool(true)
93+
TEST PASSED: Message sent and deleted OK

‎win32/sendmail.c

Lines changed: 86 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,37 @@ PHPAPI char *GetSMErrorText(int index)
333333
}
334334
}
335335

336+
/* strtok_r like, but recognizes quoted-strings */
337+
static char *find_address(char *list, char **state)
338+
{
339+
zend_bool in_quotes = 0;
340+
char *p = list;
341+
342+
if (list == NULL) {
343+
if (*state == NULL) {
344+
return NULL;
345+
}
346+
p = list = *state;
347+
}
348+
*state = NULL;
349+
while ((p = strpbrk(p, ",\"\\")) != NULL) {
350+
if (*p == '\\' && in_quotes) {
351+
if (p[1] == '\0') {
352+
/* invalid address; let SMTP server deal with it */
353+
break;
354+
}
355+
p++;
356+
} else if (*p == '"') {
357+
in_quotes = !in_quotes;
358+
} else if (*p == ',' && !in_quotes) {
359+
*p = '\0';
360+
*state = p + 1;
361+
break;
362+
}
363+
p++;
364+
}
365+
return list;
366+
}
336367

337368
/*********************************************************************
338369
// Name: SendText
@@ -357,7 +388,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
357388
{
358389
int res;
359390
char *p;
360-
char *tempMailTo, *token, *pos1, *pos2;
391+
char *tempMailTo, *token, *token_state, *pos1, *pos2;
361392
char *server_response = NULL;
362393
char *stripped_header = NULL;
363394
zend_string *data_cln;
@@ -410,7 +441,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
410441

411442
tempMailTo = estrdup(mailTo);
412443
/* Send mail to all rcpt's */
413-
token = strtok(tempMailTo, ",");
444+
token = find_address(tempMailTo, &token_state);
414445
while (token != NULL)
415446
{
416447
SMTP_SKIP_SPACE(token);
@@ -424,14 +455,14 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
424455
efree(tempMailTo);
425456
return (res);
426457
}
427-
token = strtok(NULL, ",");
458+
token = find_address(NULL, &token_state);
428459
}
429460
efree(tempMailTo);
430461

431462
if (mailCc && *mailCc) {
432463
tempMailTo = estrdup(mailCc);
433464
/* Send mail to all rcpt's */
434-
token = strtok(tempMailTo, ",");
465+
token = find_address(tempMailTo, &token_state);
435466
while (token != NULL)
436467
{
437468
SMTP_SKIP_SPACE(token);
@@ -445,7 +476,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
445476
efree(tempMailTo);
446477
return (res);
447478
}
448-
token = strtok(NULL, ",");
479+
token = find_address(NULL, &token_state);
449480
}
450481
efree(tempMailTo);
451482
}
@@ -471,7 +502,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
471502
tempMailTo = estrndup(pos1, pos2 - pos1);
472503
}
473504

474-
token = strtok(tempMailTo, ",");
505+
token = find_address(tempMailTo, &token_state);
475506
while (token != NULL)
476507
{
477508
SMTP_SKIP_SPACE(token);
@@ -485,7 +516,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
485516
efree(tempMailTo);
486517
return (res);
487518
}
488-
token = strtok(NULL, ",");
519+
token = find_address(NULL,&token_state);
489520
}
490521
efree(tempMailTo);
491522
}
@@ -496,7 +527,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
496527
if (mailBcc && *mailBcc) {
497528
tempMailTo = estrdup(mailBcc);
498529
/* Send mail to all rcpt's */
499-
token = strtok(tempMailTo, ",");
530+
token = find_address(tempMailTo, &token_state);
500531
while (token != NULL)
501532
{
502533
SMTP_SKIP_SPACE(token);
@@ -510,7 +541,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
510541
efree(tempMailTo);
511542
return (res);
512543
}
513-
token = strtok(NULL, ",");
544+
token = find_address(NULL, &token_state);
514545
}
515546
efree(tempMailTo);
516547
}
@@ -544,7 +575,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
544575
}
545576
}
546577

547-
token = strtok(tempMailTo, ",");
578+
token = find_address(tempMailTo, &token_state);
548579
while (token != NULL)
549580
{
550581
SMTP_SKIP_SPACE(token);
@@ -558,7 +589,7 @@ static int SendText(char *RPath, char *Subject, char *mailTo, char *mailCc, char
558589
efree(tempMailTo);
559590
return (res);
560591
}
561-
token = strtok(NULL, ",");
592+
token = find_address(NULL, &token_state);
562593
}
563594
efree(tempMailTo);
564595

@@ -978,6 +1009,46 @@ static unsigned long GetAddr(LPSTR szHost)
9781009
return (lAddr);
9791010
} /* end GetAddr() */
9801011

1012+
/* returns the contents of an angle-addr (caller needs to efree) or NULL */
1013+
static char *get_angle_addr(char *address)
1014+
{
1015+
zend_bool in_quotes = 0;
1016+
char *p1 = address, *p2;
1017+
1018+
while ((p1 = strpbrk(p1, "<\"\\")) != NULL) {
1019+
if (*p1 == '\\' && in_quotes) {
1020+
if (p1[1] == '\0') {
1021+
/* invalid address; let SMTP server deal with it */
1022+
return NULL;
1023+
}
1024+
p1++;
1025+
} else if (*p1 == '"') {
1026+
in_quotes = !in_quotes;
1027+
} else if (*p1 == '<' && !in_quotes) {
1028+
break;
1029+
}
1030+
p1++;
1031+
}
1032+
if (p1 == NULL) return NULL;
1033+
p2 = ++p1;
1034+
while ((p2 = strpbrk(p2, ">\"\\")) != NULL) {
1035+
if (*p2 == '\\' && in_quotes) {
1036+
if (p2[1] == '\0') {
1037+
/* invalid address; let SMTP server deal with it */
1038+
return NULL;
1039+
}
1040+
p2++;
1041+
} else if (*p2 == '"') {
1042+
in_quotes = !in_quotes;
1043+
} else if (*p2 == '>' && !in_quotes) {
1044+
break;
1045+
}
1046+
p2++;
1047+
}
1048+
if (p2 == NULL) return NULL;
1049+
1050+
return estrndup(p1, p2 - p1);
1051+
}
9811052

9821053
/*********************************************************************
9831054
// Name: int FormatEmailAddress
@@ -993,13 +1064,12 @@ static unsigned long GetAddr(LPSTR szHost)
9931064
// History:
9941065
//********************************************************************/
9951066
static int FormatEmailAddress(char* Buf, char* EmailAddress, char* FormatString) {
996-
char *tmpAddress1, *tmpAddress2;
1067+
char *tmpAddress;
9971068
int result;
9981069

999-
if( (tmpAddress1 = strchr(EmailAddress, '<')) && (tmpAddress2 = strchr(tmpAddress1, '>')) ) {
1000-
*tmpAddress2 = 0; // terminate the string temporarily.
1001-
result = snprintf(Buf, MAIL_BUFFER_SIZE, FormatString , tmpAddress1+1);
1002-
*tmpAddress2 = '>'; // put it back the way it was.
1070+
if ((tmpAddress = get_angle_addr(EmailAddress)) != NULL) {
1071+
result = snprintf(Buf, MAIL_BUFFER_SIZE, FormatString, tmpAddress);
1072+
efree(tmpAddress);
10031073
return result;
10041074
}
10051075
return snprintf(Buf, MAIL_BUFFER_SIZE , FormatString , EmailAddress );

0 commit comments

Comments
 (0)