CVE-2019-11458: Deserialization of Untrusted Data (CWE-502)
value: An issue was discovered in SmtpTransport in CakePHP 3.7.6. An unserialized object with modified internal properties can trigger arbitrary file overwriting upon destruction.
CWE description: The product deserializes untrusted data without sufficiently verifying that the resulting data will be valid.
CVSS v3 base score: 7.5
Published: 2019-05-08T18:29Z
Repository: https://github.com/cakephp/cakephp
Language: PHP
Files changed: src/Mailer/Transport/SmtpTransport.php, tests/TestCase/Mailer/Transport/SmtpTransportTest.php
Commit date: 2019-04-22 22:15:20 -0400
Commit message: Fix side-effect in destructor

SmtpTransport had the potential to create a harmful side-effect
in its destructor should an untrusted value ever be deserialized. This
solves that by removing the socket on wakeup.

Diff: diff --git a/src/Mailer/Transport/SmtpTransport.php b/src/Mailer/Transport/SmtpTransport.php
index ed17955301..825fe27454 100644
--- a/src/Mailer/Transport/SmtpTransport.php
+++ b/src/Mailer/Transport/SmtpTransport.php
@@ -76,10 +76,22 @@ class SmtpTransport extends AbstractTransport
         } catch (Exception $e) {
             // avoid fatal error on script termination
         }
     }
 
+    /**
+     * Unserialize handler.
+     *
+     * Ensure that the socket property isn't reinitialized in a broken state.
+     *
+     * @return void
+     */
+    public function __wakeup()
+    {
+        $this->_socket = null;
+    }
+
     /**
      * Connect to the SMTP server.
      *
      * This method tries to connect only in case there is no open
      * connection available already.
diff --git a/tests/TestCase/Mailer/Transport/SmtpTransportTest.php b/tests/TestCase/Mailer/Transport/SmtpTransportTest.php
index 4189d5d2e6..76732d75e9 100644
--- a/tests/TestCase/Mailer/Transport/SmtpTransportTest.php
+++ b/tests/TestCase/Mailer/Transport/SmtpTransportTest.php
@@ -707,6 +707,27 @@ class SmtpTransportTest extends TestCase
         $this->socket->expects($this->at(12))->method('write')->with("QUIT\r\n");
         $this->socket->expects($this->at(13))->method('disconnect');
 
         $this->SmtpTransport->send($email);
     }
+
+    /**
+     * Ensure that unserialized transports have no connection.
+     *
+     * @return void
+     */
+    public function testSerializeCleanupSocket()
+    {
+        $this->socket->expects($this->at(0))->method('connect')->will($this->returnValue(true));
+        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n"));
+        $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n");
+        $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 OK\r\n"));
+
+        $smtpTransport = new SmtpTestTransport();
+        $smtpTransport->setSocket($this->socket);
+        $smtpTransport->connect();
+
+        $result = unserialize(serialize($smtpTransport));
+        $this->assertAttributeEquals(null, '_socket', $result);
+        $this->assertFalse($result->connected());
+    }
 }

Fixed code:     /**
     * Unserialize handler.
     *
     * Ensure that the socket property isn't reinitialized in a broken state.
     *
     * @return void
     */
    public function __wakeup()
    {
        $this->_socket = null;
    }


    /**
     * Ensure that unserialized transports have no connection.
     *
     * @return void
     */
    public function testSerializeCleanupSocket()
    {
        $this->socket->expects($this->at(0))->method('connect')->will($this->returnValue(true));
        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n"));
        $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n");
        $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 OK\r\n"));

        $smtpTransport = new SmtpTestTransport();
        $smtpTransport->setSocket($this->socket);
        $smtpTransport->connect();

        $result = unserialize(serialize($smtpTransport));
        $this->assertAttributeEquals(null, '_socket', $result);
        $this->assertFalse($result->connected());
    }