Always escape shell arguments before mail()

src/Transport/Sendmail.php

@@ -257,15 +257,15 @@ protected function prepareParameters(Mail\Message $message)
 
         $sender = $message->getSender();
         if ($sender instanceof AddressInterface) {
-            $parameters .= ' -f' . $sender->getEmail();
+            $parameters .= ' -f' . \escapeshellarg($sender->getEmail());
             return $parameters;
         }
 
         $from = $message->getFrom();
         if (count($from)) {
             $from->rewind();
             $sender      = $from->current();
-            $parameters .= ' -f' . $sender->getEmail();
+            $parameters .= ' -f' . \escapeshellarg($sender->getEmail());
             return $parameters;
         }
 

test/MessageTest.php

@@ -12,6 +12,7 @@
 use stdClass;
 use Zend\Mail\Address;
 use Zend\Mail\AddressList;
+use Zend\Mail\Exception;
 use Zend\Mail\Header;
 use Zend\Mail\Headers;
 use Zend\Mail\Message;
@@ -853,4 +854,14 @@ public function testMailHeaderContainsZeroValue()
         $msg = Message::fromString($message);
         $this->assertContains('X-Spam-Score: 0', $msg->toString());
     }
+
+    /**
+     * @ref CVE-2016-10033 which targeted WordPress
+     */
+    public function testSecondCodeInjectionInFromHeader()
+    {
+        $message = new Message();
+        $this->setExpectedException(Exception\InvalidArgumentException::class);
+        $message->setFrom('user@xenial(tmp1 -be ${run{${substr{0}{1}{$spool_directory}}usr${substr{0}{1}{$spool_directory}}bin${substr{0}{1}{$spool_directory}}touch${substr{10}{1}{$tod_log}}${substr{0}{1}{$spool_directory}}tmp${substr{0}{1}{$spool_directory}}test}}  tmp2)', 'Sender\'s name');
+    }
 }

test/Transport/SendmailTest.php

@@ -9,6 +9,11 @@
 
 namespace ZendTest\Mail\Transport;
 
+use ReflectionMethod;
+use Zend\Mail\Address\AddressInterface;
+use Zend\Mail\AddressList;
+use Zend\Mail\Header;
+use Zend\Mail\Headers;
 use Zend\Mail\Message;
 use Zend\Mail\Transport\Exception\RuntimeException;
 use Zend\Mail\Transport\Sendmail;
@@ -88,7 +93,7 @@ public function testReceivesMailArtifactsOnUnixSystems()
         $this->assertContains("From: [email protected],\n Matthew <[email protected]>\n", $this->additional_headers);
         $this->assertContains("X-Foo-Bar: Matthew\n", $this->additional_headers);
         $this->assertContains("Sender: Ralph Schindler <[email protected]>\n", $this->additional_headers);
-        $this->assertEquals('-R hdrs -fralph[email protected]', $this->additional_parameters);
+        $this->assertEquals('-R hdrs -f\'ralph[email protected]\'', $this->additional_parameters);
     }
 
     public function testReceivesMailArtifactsOnWindowsSystems()
@@ -158,4 +163,53 @@ public function testValidEmailLocaDomainInFromHeader()
         $this->transport->send($message);
         $this->assertContains('From: Foo Bar <"foo-bar"@domain>', $this->additional_headers);
     }
+
+    /**
+     * @ref CVE-2016-10033 which targeted WordPress
+     */
+    public function testPrepareParametersEscapesSenderUsingEscapeShellArg()
+    {
+        // @codingStandardsIgnoreStart
+        $injectedEmail = 'user@xenial(tmp1 -be ${run{${substr{0}{1}{$spool_directory}}usr${substr{0}{1}{$spool_directory}}bin${substr{0}{1}{$spool_directory}}touch${substr{10}{1}{$tod_log}}${substr{0}{1}{$spool_directory}}tmp${substr{0}{1}{$spool_directory}}test}}  tmp2)';
+        // @codingStandardsIgnoreEnd
+
+        $sender = $this->prophesize(AddressInterface::class);
+        $sender->getEmail()->willReturn($injectedEmail);
+
+        $message = $this->prophesize(Message::class);
+        $message->getSender()->will([$sender, 'reveal']);
+        $message->getFrom()->shouldNotBeCalled();
+
+        $r = new ReflectionMethod($this->transport, 'prepareParameters');
+        $r->setAccessible(true);
+
+        $parameters = $r->invoke($this->transport, $message->reveal());
+        $this->assertEquals(' -f' . escapeshellarg($injectedEmail), $parameters);
+    }
+
+    /**
+     * @ref CVE-2016-10033 which targeted WordPress
+     */
+    public function testPrepareParametersEscapesFromAddressUsingEscapeShellArg()
+    {
+        // @codingStandardsIgnoreStart
+        $injectedEmail = 'user@xenial(tmp1 -be ${run{${substr{0}{1}{$spool_directory}}usr${substr{0}{1}{$spool_directory}}bin${substr{0}{1}{$spool_directory}}touch${substr{10}{1}{$tod_log}}${substr{0}{1}{$spool_directory}}tmp${substr{0}{1}{$spool_directory}}test}}  tmp2)';
+        // @codingStandardsIgnoreEnd
+
+        $address = $this->prophesize(AddressInterface::class);
+        $address->getEmail()->willReturn($injectedEmail)->shouldBeCalledTimes(2);
+
+        $from = new AddressList();
+        $from->add($address->reveal());
+
+        $message = $this->prophesize(Message::class);
+        $message->getSender()->willReturn(null);
+        $message->getFrom()->willReturn($from);
+
+        $r = new ReflectionMethod($this->transport, 'prepareParameters');
+        $r->setAccessible(true);
+
+        $parameters = $r->invoke($this->transport, $message->reveal());
+        $this->assertEquals(' -f' . escapeshellarg($injectedEmail), $parameters);
+    }
 }