diff --git a/SECURITY.md b/SECURITY.md index 5e917cd04..fc3e61c20 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 f8c5c2ccd..3af67b5cb 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 50b777c05..50366b439 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 51dff9217..ed14d7c7a 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 @@ public function createBody() $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 @@ protected function attachAll($disposition_type, $boundary) //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 @@ protected function attachAll($disposition_type, $boundary) $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 @@ protected function encodeFile($path, $encoding = self::ENCODING_BASE64) if ($this->exceptions) { throw $exc; } + return ''; } } @@ -4727,6 +4718,28 @@ public static function hasLineLongerThanMax($str) 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 cd6fc2f2e..7d4c88f6c 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 bc08b271f..aa5555149 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 b4c91f51d..4879a171b 100644 --- a/test/PHPMailerTest.php +++ b/test/PHPMailerTest.php @@ -1214,7 +1214,7 @@ public function testHTMLAttachment() } //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 @@ public function testHTMLAttachment() $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 @@ public function testAddressSplitting() ['name' => 'Jill User', 'address' => 'jill@example.net'], ['name' => '', 'address' => 'frank@example.com'], ], - $this->Mail->parseAddresses( + PHPMailer::parseAddresses( 'Joe User ,' . 'Jill User ,' . 'frank@example.com,'