diff --git a/src/PHPMailer.php b/src/PHPMailer.php index ede13d5c..a8dff0a4 100644 --- a/src/PHPMailer.php +++ b/src/PHPMailer.php @@ -220,6 +220,13 @@ class PHPMailer */ public $Sendmail = '/usr/sbin/sendmail'; + /** + * Parsed sendmail parameters extracted from sendmail_path. + * + * @var array + */ + private $SendmailParams = []; + /** * Whether mail() uses a fully sendmail-compatible MTA. * One which supports sendmail's "-oi -f" options. @@ -988,6 +995,57 @@ class PHPMailer $this->Mailer = 'mail'; } + /** + * Extract supported sendmail parameters from a path string. + * + * @param string $sendmailPath + * + * @return string The sendmail path without the parsed parameters + */ + private function parseSendmailPath($sendmailPath) + { + $sendmailPath = trim((string)$sendmailPath); + if ($sendmailPath === '') { + return $sendmailPath; + } + + $parts = preg_split('/\s+/', $sendmailPath); + if (empty($parts)) { + return $sendmailPath; + } + + $command = array_shift($parts); + $remainder = []; + + // Parse only -t, -i, and -f parameters. + for ($i = 0; $i < count($parts); ++$i) { + $part = $parts[$i]; + if (preg_match('/^-(i|t)$/', $part, $matches)) { + $this->SendmailParams[$matches[0]] = ''; + continue; + } + if (preg_match('/^-f(.*)$/', $part, $matches)) { + $address = $matches[1]; + if ($address === '' && isset($parts[$i + 1]) && strpos($parts[$i + 1], '-') !== 0) { + $address = $parts[++$i]; + } + if (static::validateAddress($address)) { + $this->SendmailParams['-f'] = $address; + continue; + } + } + + $remainder[] = $part; + } + + // The params that are not parsed are added back to the command. + if (!empty($remainder)) { + $command .= ' ' . implode(' ', $remainder); + } + + return $command; + } + /** * Send messages using $Sendmail. */ @@ -996,10 +1054,9 @@ class PHPMailer $ini_sendmail_path = ini_get('sendmail_path'); if (false === stripos($ini_sendmail_path, 'sendmail')) { - $this->Sendmail = '/usr/sbin/sendmail'; - } else { - $this->Sendmail = $ini_sendmail_path; + $ini_sendmail_path = '/usr/sbin/sendmail'; } + $this->Sendmail = $this->parseSendmailPath($ini_sendmail_path); $this->Mailer = 'sendmail'; } @@ -1011,10 +1068,9 @@ class PHPMailer $ini_sendmail_path = ini_get('sendmail_path'); if (false === stripos($ini_sendmail_path, 'qmail')) { - $this->Sendmail = '/var/qmail/bin/qmail-inject'; - } else { - $this->Sendmail = $ini_sendmail_path; + $ini_sendmail_path = '/var/qmail/bin/qmail-inject'; } + $this->Sendmail = $this->parseSendmailPath($ini_sendmail_path); $this->Mailer = 'qmail'; } @@ -1860,29 +1916,34 @@ class PHPMailer //PHP config has a sender address we can use $this->Sender = ini_get('sendmail_from'); } - //CVE-2016-10033, CVE-2016-10045: Don't pass -f if characters will be escaped. - if (!empty($this->Sender) && static::validateAddress($this->Sender) && self::isShellSafe($this->Sender)) { - if ($this->Mailer === 'qmail') { - $sendmailFmt = '%s -f%s'; - } else { - if (strpos($this->Sendmail, '-f') === false) { - $sendmailFmt = '%s -f%s -oi -t'; - } else { - $sendmailFmt = '%s -oi -t'; - } - } - } elseif ($this->Mailer === 'qmail') { - $sendmailFmt = '%s'; - } else { - //Allow sendmail to choose a default envelope sender. It may - //seem preferable to force it to use the From header as with - //SMTP, but that introduces new problems (see - //), and - //it has historically worked this way. - $sendmailFmt = '%s -oi -t'; + + $sendmailArgs = []; + + // CVE-2016-10033, CVE-2016-10045: Don't pass -f if characters will be escaped. + // Also don't add the -f automatically unless it has been set either via Sender + // or sendmail_path. Otherwise it can introduce new problems. + // @see http://github.com/PHPMailer/PHPMailer/issues/2298 + if (isset($this->SendmailParams['-f']) && self::isShellSafe($this->SendmailParams['-f'])) { + $sendmailArgs[] = '-f' . $this->SendmailParams['-f']; + } elseif (!empty($this->Sender) && static::validateAddress($this->Sender) && self::isShellSafe($this->Sender)) { + $sendmailArgs[] = '-f' . $this->Sender; } - $sendmail = sprintf($sendmailFmt, escapeshellcmd($this->Sendmail), $this->Sender); + // Qmail doesn't accept all the sendmail parameters + // @see https://github.com/PHPMailer/PHPMailer/issues/3189 + if ($this->Mailer !== 'qmail') { + if (isset($this->SendmailParams['-i'])) { + $sendmailArgs[] = '-i'; + } + + if (isset($this->SendmailParams['-t'])) { + $sendmailArgs[] = '-t'; + } + } + + $resultArgs = (empty($sendmailArgs) ? '' : ' ' . implode(' ', $sendmailArgs)); + + $sendmail = trim(escapeshellcmd($this->Sendmail) . $resultArgs); $this->edebug('Sendmail path: ' . $this->Sendmail); $this->edebug('Sendmail command: ' . $sendmail); $this->edebug('Envelope sender: ' . $this->Sender); @@ -2066,8 +2127,8 @@ class PHPMailer $this->Sender = ini_get('sendmail_from'); } if (!empty($this->Sender) && static::validateAddress($this->Sender)) { - $phpmailer_path = ini_get('sendmail_path'); - if (self::isShellSafe($this->Sender) && strpos($phpmailer_path, '-f') === false) { + $this->parseSendmailPath(ini_get('sendmail_path')); + if (self::isShellSafe($this->Sender) && !isset($this->SendmailParams['-f'])) { $params = sprintf('-f%s', $this->Sender); } $old_from = ini_get('sendmail_from'); diff --git a/test/PHPMailer/MailTransportTest.php b/test/PHPMailer/MailTransportTest.php index 69327c8e..b0c86af3 100644 --- a/test/PHPMailer/MailTransportTest.php +++ b/test/PHPMailer/MailTransportTest.php @@ -174,4 +174,74 @@ final class MailTransportTest extends SendTestCase self::assertTrue($this->Mail->send(), $this->Mail->ErrorInfo); } + + /** + * Test parsing of sendmail path and with certain parameters. + * + * @group sendmailparams + * @covers \PHPMailer\PHPMailer\PHPMailer::parseSendmailPath + * @dataProvider sendmailPathProvider + * + * @param string $sendmailPath The sendmail path to parse. + * @param string $expectedCommand The expected command after parsing. + * @param array $expectedParams The expected parameters after parsing. + */ + public function testParseSendmailPath($sendmailPath, $expectedCommand, array $expectedParams) + { + $mailer = $this->Mail; + + $command = \Closure::bind( + function ($path) { + return $this->{'parseSendmailPath'}($path); + }, + $mailer, + \PHPMailer\PHPMailer\PHPMailer::class + )($sendmailPath); + + $params = \Closure::bind( + function () { + return $this->{'SendmailParams'}; + }, + $mailer, + \PHPMailer\PHPMailer\PHPMailer::class + )(); + + self::assertSame($expectedCommand, $command); + self::assertSame($expectedParams, $params); + } + + /** + * Data provider for testParseSendmailPath. + */ + + public function sendmailPathProvider() + { + return [ + 'path only' => [ + '/usr/sbin/sendmail', + '/usr/sbin/sendmail', + [], + ], + 'with i and t' => [ + '/usr/sbin/sendmail -i -t', + '/usr/sbin/sendmail', + ['-i' => '', '-t' => ''], + ], + 'with f concatenated' => [ + '/usr/sbin/sendmail -frpath@example.org -i', + '/usr/sbin/sendmail', + ['-f' => 'rpath@example.org', '-i' => ''], + ], + 'with f separated' => [ + '/usr/sbin/sendmail -f rpath@example.org -t', + '/usr/sbin/sendmail', + ['-f' => 'rpath@example.org', '-t' => ''], + ], + 'with extra flags preserved' => [ + '/opt/sendmail -fuser@example.org -x -y', + '/opt/sendmail -x -y', + ['-f' => 'user@example.org'], + ], + ]; + } }