Permalink
Browse files

Merge PR #930 from @Zanaxer

2 parents 8d68507 + 833c35f commit 06aae8b651aa4ae3d4d2f66390afde66ceae98b7 @Synchro Synchro committed Dec 28, 2016
Showing with 44 additions and 9 deletions.
  1. +44 −9 class.phpmailer.php
View
@@ -1364,21 +1364,24 @@ public function postSend()
*/
protected function sendmailSend($header, $body)
{
- // CVE-2016-10033, CVE-2016-10045: Don't pass -f if characters will be escaped
- // by escapeshellcmd when popen is called due to safe mode.
- if (!empty($this->Sender) && !ini_get('safe_mode')) {
+ // CVE-2016-10033, CVE-2016-10045: Don't pass -f if characters will be escaped.
+ if (!empty($this->Sender) and self::isShellSafe($this->Sender)) {
if ($this->Mailer == 'qmail') {
- $sendmail = sprintf('%s -f%s', escapeshellcmd($this->Sendmail), escapeshellarg($this->Sender));
+ $sendmailFmt = '%s -f%s';
} else {
- $sendmail = sprintf('%s -oi -f%s -t', escapeshellcmd($this->Sendmail), escapeshellarg($this->Sender));
+ $sendmailFmt = '%s -oi -f%s -t';
}
} else {
if ($this->Mailer == 'qmail') {
- $sendmail = sprintf('%s', escapeshellcmd($this->Sendmail));
+ $sendmailFmt = '%s';
} else {
- $sendmail = sprintf('%s -oi -t', escapeshellcmd($this->Sendmail));
+ $sendmailFmt = '%s -oi -t';
}
}
+
+ // TODO: If possible, this should be changed to escapeshellarg. Needs thorough testing.
+ $sendmail = sprintf($sendmailFmt, escapeshellcmd($this->Sendmail), $this->Sender);
+
if ($this->SingleTo) {
foreach ($this->SingleToArray as $toAddr) {
if (!@$mail = popen($sendmail, 'w')) {
@@ -1425,6 +1428,38 @@ protected function sendmailSend($header, $body)
}
/**
+ * Fix CVE-2016-10033 and CVE-2016-10045 by disallowing potentially unsafe shell characters.
+ *
+ * Note that escapeshellarg and escapeshellcmd are inadequate for our purposes, especially on Windows.
+ * @param string $string The string to be validated
+ * @see https://github.com/PHPMailer/PHPMailer/issues/924 CVE-2016-10045 bug report
+ * @access protected
+ * @return boolean
+ */
+ protected static function isShellSafe($string)
+ {
+ // Future-proof
+ if (escapeshellcmd($string) !== $string or !in_array(escapeshellarg($string), array("'$string'", "\"$string\""))) {
+ return false;
+ }
+
+ $length = strlen($string);
+
+ for ($i = 0; $i < $length; $i++) {
+ $c = $string[$i];
+
+ // All other characters have a special meaning in at least one common shell, including = and +.
+ // Full stop (.) has a special meaning in cmd.exe, but its impact should be negligible here.
+ // Note that this does permit non-Latin alphanumeric characters based on the current locale.
+ if (!ctype_alnum($c) && strpos('@_-.', $c) === false) {
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ /**
* Send mail using the PHP mail() function.
* @param string $header The message headers
* @param string $body The message body
@@ -1445,8 +1480,8 @@ protected function mailSend($header, $body)
//This sets the SMTP envelope sender which gets turned into a return-path header by the receiver
if (!empty($this->Sender) and $this->validateAddress($this->Sender)) {
// CVE-2016-10033, CVE-2016-10045: Don't pass -f if characters will be escaped.
- if (escapeshellcmd($this->Sender) === $this->Sender && in_array(escapeshellarg($this->Sender), array("'$this->Sender'", "\"$this->Sender\""))) {
- $params = sprintf('-f%s', escapeshellarg($this->Sender));
+ if (self::isShellSafe($this->Sender)) {
+ $params = sprintf('-f%s', $this->Sender);
}
}
if (!empty($this->Sender) and !ini_get('safe_mode') and $this->validateAddress($this->Sender)) {

0 comments on commit 06aae8b

Please sign in to comment.