From c2796cb1cb99d7717290b48c4e6f32cb6c60b7b3 Mon Sep 17 00:00:00 2001 From: Marcus Bointon Date: Wed, 27 May 2020 14:24:03 +0200 Subject: [PATCH] Merge pull request from GHSA-f7hx-fqxw-rvvj * Initial fixes, tests, and bump to 6.1.6 * Add CVE number --- SECURITY.md | 2 ++ VERSION | 2 +- changelog.md | 4 ++- src/PHPMailer.php | 47 +++++++++++++++++++++------------ src/POP3.php | 2 +- src/SMTP.php | 2 +- test/PHPMailerTest.php | 60 ++++++++++++++++++++++++++++++++++++++++-- 7 files changed, 96 insertions(+), 23 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 5e917cd0..fc3e61c2 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -2,6 +2,8 @@ Please disclose any vulnerabilities found responsibly - report any security problems found to the maintainers privately. +PHPMailer versions 6.1.5 and earlier contain an output escaping bug that occurs in `Content-Type` and `Content-Disposition` when filenames passed into `addAttachment` and other methods that accept attachment names contain double quote characters, in contravention of RFC822 3.4.1. No specific vulnerability has been found relating to this, but it could allow file attachments to bypass attachment filters that are based on matching filename extensions. Recorded as [CVE-2020-13625](https://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2020-13625). Reported by Elar Lang of Clarified Security. + PHPMailer versions prior to 6.0.6 and 5.2.27 are vulnerable to an object injection attack by passing `phar://` paths into `addAttachment()` and other functions that may receive unfiltered local paths, possibly leading to RCE. Recorded as [CVE-2018-19296](https://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2018-19296). See [this article](https://knasmueller.net/5-answers-about-php-phar-exploitation) for more info on this type of vulnerability. Mitigated by blocking the use of paths containing URL-protocol style prefixes such as `phar://`. Reported by Sehun Oh of cyberone.kr. PHPMailer versions prior to 5.2.24 (released July 26th 2017) have an XSS vulnerability in one of the code examples, [CVE-2017-11503](https://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2017-11503). The `code_generator.phps` example did not filter user input prior to output. This file is distributed with a `.phps` extension, so it it not normally executable unless it is explicitly renamed, and the file is not included when PHPMailer is loaded through composer, so it is safe by default. There was also an undisclosed potential XSS vulnerability in the default exception handler (unused by default). Patches for both issues kindly provided by Patrick Monnerat of the Fedora Project. diff --git a/VERSION b/VERSION index f8c5c2cc..3af67b5c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.1.5 \ No newline at end of file +6.1.6 \ No newline at end of file diff --git a/changelog.md b/changelog.md index 50b777c0..50366b43 100644 --- a/changelog.md +++ b/changelog.md @@ -1,9 +1,11 @@ # PHPMailer Change Log +## Version 6.1.6 (May 27th, 2020) +* **SECURITY** Fix insufficient output escaping bug in file attachment names. CVE-2020-13625. Reported by Elar Lang of Clarified Security. * Correct Armenian ISO language code from `am` to `hy`, add mapping for fallback * Use correct timeout property in debug output -## Version 6.1.5 (March 14th, 2020 +## Version 6.1.5 (March 14th, 2020) * Reject invalid custom headers that are empty or contain breaks * Various fixes for DKIM issues, especially when using `mail()` transport * Drop the `l=` length tag from DKIM signatures; it's a mild security risk diff --git a/src/PHPMailer.php b/src/PHPMailer.php index 51dff921..ed14d7c7 100644 --- a/src/PHPMailer.php +++ b/src/PHPMailer.php @@ -745,7 +745,7 @@ class PHPMailer * * @var string */ - const VERSION = '6.1.5'; + const VERSION = '6.1.6'; /** * Error severity: message only, continue processing. @@ -2607,7 +2607,7 @@ class PHPMailer $altBodyEncoding = static::ENCODING_QUOTED_PRINTABLE; } //Use this as a preamble in all multipart message types - $mimepre = 'This is a multi-part message in MIME format.' . static::$LE . static::$LE; + $mimepre = 'This is a multi-part message in MIME format.' . static::$LE . static::$LE; switch ($this->message_type) { case 'inline': $body .= $mimepre; @@ -3064,9 +3064,9 @@ class PHPMailer //Only include a filename property if we have one if (!empty($name)) { $mime[] = sprintf( - 'Content-Type: %s; name="%s"%s', + 'Content-Type: %s; name=%s%s', $type, - $this->encodeHeader($this->secureHeader($name)), + static::quotedString($this->encodeHeader($this->secureHeader($name))), static::$LE ); } else { @@ -3086,24 +3086,14 @@ class PHPMailer $mime[] = 'Content-ID: <' . $this->encodeHeader($this->secureHeader($cid)) . '>' . static::$LE; } - // If a filename contains any of these chars, it should be quoted, - // but not otherwise: RFC2183 & RFC2045 5.1 - // Fixes a warning in IETF's msglint MIME checker - // Allow for bypassing the Content-Disposition header totally + // Allow for bypassing the Content-Disposition header if (!empty($disposition)) { $encoded_name = $this->encodeHeader($this->secureHeader($name)); - if (preg_match('/[ ()<>@,;:"\/\[\]?=]/', $encoded_name)) { - $mime[] = sprintf( - 'Content-Disposition: %s; filename="%s"%s', - $disposition, - $encoded_name, - static::$LE . static::$LE - ); - } elseif (!empty($encoded_name)) { + if (!empty($encoded_name)) { $mime[] = sprintf( 'Content-Disposition: %s; filename=%s%s', $disposition, - $encoded_name, + static::quotedString($encoded_name), static::$LE . static::$LE ); } else { @@ -3163,6 +3153,7 @@ class PHPMailer if ($this->exceptions) { throw $exc; } + return ''; } } @@ -4727,6 +4718,28 @@ class PHPMailer return (bool) preg_match('/^(.{' . (self::MAX_LINE_LENGTH + strlen(static::$LE)) . ',})/m', $str); } + /** + * If a string contains any "special" characters, double-quote the name, + * and escape any double quotes with a backslash. + * + * @param string $str + * + * @return string + * + * @see RFC822 3.4.1 + */ + public static function quotedString($str) + { + if (preg_match('/[ ()<>@,;:"\/\[\]?=]/', $str)) { + //If the string contains any of these chars, it must be double-quoted + //and any double quotes must be escaped with a backslash + return '"' . str_replace('"', '\\"', $str) . '"'; + } + + //Return the string untouched, it doesn't need quoting + return $str; + } + /** * Allows for public read access to 'to' property. * Before the send() call, queued addresses (i.e. with IDN) are not yet included. diff --git a/src/POP3.php b/src/POP3.php index cd6fc2f2..7d4c88f6 100644 --- a/src/POP3.php +++ b/src/POP3.php @@ -45,7 +45,7 @@ class POP3 * * @var string */ - const VERSION = '6.1.5'; + const VERSION = '6.1.6'; /** * Default POP3 port number. diff --git a/src/SMTP.php b/src/SMTP.php index bc08b271..aa555514 100644 --- a/src/SMTP.php +++ b/src/SMTP.php @@ -34,7 +34,7 @@ class SMTP * * @var string */ - const VERSION = '6.1.5'; + const VERSION = '6.1.6'; /** * SMTP line break constant. diff --git a/test/PHPMailerTest.php b/test/PHPMailerTest.php index b4c91f51..4879a171 100644 --- a/test/PHPMailerTest.php +++ b/test/PHPMailerTest.php @@ -1214,7 +1214,7 @@ EOT; } //Make sure that trying to attach a nonexistent file fails - $filename = __FILE__ . md5(microtime()). 'nonexistent_file.txt'; + $filename = __FILE__ . md5(microtime()) . 'nonexistent_file.txt'; $this->assertFalse($this->Mail->addAttachment($filename)); //Make sure that trying to attach an existing but unreadable file fails touch($filename); @@ -1227,6 +1227,62 @@ EOT; $this->assertTrue($this->Mail->send(), $this->Mail->ErrorInfo); } + /** + * Attachment naming test. + */ + public function testAttachmentNaming() + { + $this->Mail->Body = 'Attachments.'; + $this->Mail->Subject .= ': Attachments'; + $this->Mail->isHTML(true); + $this->Mail->CharSet = 'UTF-8'; + $this->Mail->addAttachment( + realpath($this->INCLUDE_DIR . '/examples/images/phpmailer_mini.png'), + 'phpmailer_mini.png";.jpg' + ); + $this->Mail->addAttachment( + realpath($this->INCLUDE_DIR . '/examples/images/phpmailer.png'), + 'phpmailer.png' + ); + $this->Mail->addAttachment( + realpath($this->INCLUDE_DIR . '/examples/images/PHPMailer card logo.png'), + 'PHPMailer card logo.png' + ); + $this->buildBody(); + $this->Mail->preSend(); + $message = $this->Mail->getSentMIMEMessage(); + $this->assertContains( + 'Content-Type: image/png; name="phpmailer_mini.png\";.jpg"', + $message, + 'Name containing double quote should be escaped in Content-Type' + ); + $this->assertContains( + 'Content-Disposition: attachment; filename="phpmailer_mini.png\";.jpg"', + $message, + 'Filename containing double quote should be escaped in Content-Disposition' + ); + $this->assertContains( + 'Content-Type: image/png; name=phpmailer.png', + $message, + 'Name without special chars should not be quoted in Content-Type' + ); + $this->assertContains( + 'Content-Disposition: attachment; filename=phpmailer.png', + $message, + 'Filename without special chars should not be quoted in Content-Disposition' + ); + $this->assertContains( + 'Content-Type: image/png; name="PHPMailer card logo.png"', + $message, + 'Name with spaces should be quoted in Content-Type' + ); + $this->assertContains( + 'Content-Disposition: attachment; filename="PHPMailer card logo.png"', + $message, + 'Filename with spaces should be quoted in Content-Disposition' + ); + } + /** * Test embedded image without a name. */ @@ -1687,7 +1743,7 @@ EOT; ['name' => 'Jill User', 'address' => 'jill@example.net'], ['name' => '', 'address' => 'frank@example.com'], ], - $this->Mail->parseAddresses( + PHPMailer::parseAddresses( 'Joe User ,' . 'Jill User ,' . 'frank@example.com,'