From: Markus Koschany <apo@debian.org>
Date: Sat, 21 Jan 2017 14:54:57 +0100
Subject: CVE-2016-10074

The mail transport (aka Swift_Transport_MailTransport) in Swift Mailer allowed
remote attackers to pass extra parameters to the mail command
and consequently execute arbitrary code via a \" (backslash double quote) in a
crafted e-mail address in the From, ReturnPath, or Sender header.

Bug-Debian: https://bugs.debian.org/849626
Bug-Upstream: https://github.com/swiftmailer/swiftmailer/issues/844
Origin: https://github.com/swiftmailer/swiftmailer/commit/e6ccf40d856af9598b76eb313b215eed25ae9e86
---
 lib/classes/Swift/Transport/MailTransport.php    | 52 +++++++++++++++++++++++-
 tests/unit/Swift/Transport/MailTransportTest.php | 27 ++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/lib/classes/Swift/Transport/MailTransport.php b/lib/classes/Swift/Transport/MailTransport.php
index ef3bc09..722da58 100644
--- a/lib/classes/Swift/Transport/MailTransport.php
+++ b/lib/classes/Swift/Transport/MailTransport.php
@@ -166,7 +166,7 @@ class Swift_Transport_MailTransport implements Swift_Transport
         }
 
         if ($this->_invoker->mail($to, $subject, $body, $headers,
-            sprintf($this->_extraParams, $reversePath))) {
+            $this->_formatExtraParams($this->_extraParams, $reversePath))) {
             if ($evt) {
                 $evt->setResult(Swift_Events_SendEvent::RESULT_SUCCESS);
                 $evt->setFailedRecipients($failedRecipients);
@@ -223,4 +223,54 @@ class Swift_Transport_MailTransport implements Swift_Transport
 
         return $path;
     }
+
+  /**
+   * Fix CVE-2016-10074 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
+   *
+   * @return bool
+   */
+   private function _isShellSafe($string)
+   {
+       // Future-proof
+       if (escapeshellcmd($string) !== $string || !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;
+   }
+
+   /**
+     * Return php mail extra params to use for invoker->mail.
+     *
+     * @param $extraParams
+     * @param $reversePath
+     *
+     * @return string|null
+     */
+    private function _formatExtraParams($extraParams, $reversePath)
+    {
+        if (false !== strpos($extraParams, '-f%s')) {
+            if (empty($reversePath) || false === $this->_isShellSafe($reversePath)) {
+                $extraParams = str_replace('-f%s', '', $extraParams);
+            } else {
+                $extraParams = sprintf($extraParams, $reversePath);
+            }
+        }
+        return !empty($extraParams) ? $extraParams : null;
+    }
+
 }
diff --git a/tests/unit/Swift/Transport/MailTransportTest.php b/tests/unit/Swift/Transport/MailTransportTest.php
index 89cb2d5..b36076b 100644
--- a/tests/unit/Swift/Transport/MailTransportTest.php
+++ b/tests/unit/Swift/Transport/MailTransportTest.php
@@ -84,6 +84,33 @@ class Swift_Transport_MailTransportTest extends \SwiftMailerTestCase
         $transport->send($message);
     }
 
+    public function testTransportSettingInvalidFromEmail()
+    {
+        $invoker = $this->_createInvoker();
+        $dispatcher = $this->_createEventDispatcher();
+        $transport = $this->_createTransport($invoker, $dispatcher);
+
+        $headers = $this->_createHeaders();
+        $message = $this->_createMessageWithRecipient($headers);
+
+        $message->shouldReceive('getReturnPath')
+            ->zeroOrMoreTimes()
+            ->andReturn(
+                '"attacker\" -oQ/tmp/ -X/var/www/cache/phpcode.php "@email.com'
+            );
+        $message->shouldReceive('getSender')
+            ->zeroOrMoreTimes()
+            ->andReturn(null);
+        $message->shouldReceive('getFrom')
+            ->zeroOrMoreTimes()
+            ->andReturn(null);
+        $invoker->shouldReceive('mail')
+            ->once()
+            ->with(\Mockery::any(), \Mockery::any(), \Mockery::any(), \Mockery::any(), null);
+
+        $transport->send($message);
+    }
+
     public function testTransportUsesHeadersFromMessage()
     {
         $invoker = $this->_createInvoker();