Browse Source

Merge branch '3.x' into 3.next

Corey Taylor 5 years ago
parent
commit
ce0fb0ef10

+ 3 - 0
src/Controller/Component/SecurityComponent.php

@@ -360,6 +360,9 @@ class SecurityComponent extends Component
         if (!isset($check['_Token']['fields'])) {
             throw new AuthSecurityException(sprintf($message, '_Token.fields'));
         }
+        if (!is_string($check['_Token']['fields'])) {
+            throw new AuthSecurityException("'_Token.fields' was invalid.");
+        }
         if (!isset($check['_Token']['unlocked'])) {
             throw new AuthSecurityException(sprintf($message, '_Token.unlocked'));
         }

+ 1 - 1
src/Http/Middleware/CsrfProtectionMiddleware.php

@@ -105,7 +105,7 @@ class CsrfProtectionMiddleware
         $cookies = $request->getCookieParams();
         $cookieData = Hash::get($cookies, $this->_config['cookieName']);
 
-        if (strlen($cookieData) > 0) {
+        if (is_string($cookieData) && strlen($cookieData) > 0) {
             $params = $request->getAttribute('params');
             $params['_csrfToken'] = $cookieData;
             $request = $request->withAttribute('params', $params);

+ 59 - 17
src/Mailer/Transport/SmtpTransport.php

@@ -257,24 +257,66 @@ class SmtpTransport extends AbstractTransport
      */
     protected function _auth()
     {
-        if (isset($this->_config['username'], $this->_config['password'])) {
-            $replyCode = (string)$this->_smtpSend('AUTH LOGIN', '334|500|502|504');
-            if ($replyCode === '334') {
-                try {
-                    $this->_smtpSend(base64_encode($this->_config['username']), '334');
-                } catch (SocketException $e) {
-                    throw new SocketException('SMTP server did not accept the username.', null, $e);
-                }
-                try {
-                    $this->_smtpSend(base64_encode($this->_config['password']), '235');
-                } catch (SocketException $e) {
-                    throw new SocketException('SMTP server did not accept the password.', null, $e);
-                }
-            } elseif ($replyCode === '504') {
-                throw new SocketException('SMTP authentication method not allowed, check if SMTP server requires TLS.');
-            } else {
-                throw new SocketException('AUTH command not recognized or not implemented, SMTP server may not require authentication.');
+        if (!isset($this->_config['username'], $this->_config['password'])) {
+            return;
+        }
+
+        $username = $this->_config['username'];
+        $password = $this->_config['password'];
+
+        $replyCode = $this->_authPlain($username, $password);
+        if ($replyCode === '235') {
+            return;
+        }
+
+        $this->_authLogin($username, $password);
+    }
+
+    /**
+     * Authenticate using AUTH PLAIN mechanism.
+     *
+     * @param string $username Username.
+     * @param string $password Password.
+     * @return string|null Response code for the command.
+     */
+    protected function _authPlain($username, $password)
+    {
+        return $this->_smtpSend(
+            sprintf(
+                'AUTH PLAIN %s',
+                base64_encode(chr(0) . $username . chr(0) . $password)
+            ),
+            '235|504|534|535'
+        );
+    }
+
+    /**
+     * Authenticate using AUTH LOGIN mechanism.
+     *
+     * @param string $username Username.
+     * @param string $password Password.
+     * @return void
+     */
+    protected function _authLogin($username, $password)
+    {
+        $replyCode = $this->_smtpSend('AUTH LOGIN', '334|500|502|504');
+        if ($replyCode === '334') {
+            try {
+                $this->_smtpSend(base64_encode($username), '334');
+            } catch (SocketException $e) {
+                throw new SocketException('SMTP server did not accept the username.', null, $e);
             }
+            try {
+                $this->_smtpSend(base64_encode($password), '235');
+            } catch (SocketException $e) {
+                throw new SocketException('SMTP server did not accept the password.', null, $e);
+            }
+        } elseif ($replyCode === '504') {
+            throw new SocketException('SMTP authentication method not allowed, check if SMTP server requires TLS.');
+        } else {
+            throw new SocketException(
+                'AUTH command not recognized or not implemented, SMTP server may not require authentication.'
+            );
         }
     }
 

+ 26 - 2
tests/TestCase/Controller/Component/SecurityComponentTest.php

@@ -564,14 +564,14 @@ class SecurityComponentTest extends TestCase
     }
 
     /**
-     * testValidatePostFormHacking method
+     * testValidatePostFormTampering method
      *
      * Test that validatePost fails if any of its required fields are missing.
      *
      * @return void
      * @triggers Controller.startup $this->Controller
      */
-    public function testValidatePostFormHacking()
+    public function testValidatePostFormTampering()
     {
         $event = new Event('Controller.startup', $this->Controller);
         $this->Security->startup($event);
@@ -1137,6 +1137,30 @@ class SecurityComponentTest extends TestCase
     }
 
     /**
+     * Test that invalid types cause failures.
+     *
+     * @return void
+     */
+    public function testValidatePostFailArrayData()
+    {
+        $event = new Event('Controller.startup', $this->Controller);
+        $this->Security->startup($event);
+        $this->Controller->request = $this->Controller->request->withParsedBody([
+            'Model' => [
+                'username' => 'mark',
+                'password' => 'sekret',
+            ],
+            '_Token' => [
+                'fields' => [],
+                'unlocked' => '',
+            ],
+        ]);
+        Configure::write('debug', false);
+        $result = $this->validatePost('SecurityException', "'_Token.fields' was invalid.");
+        $this->assertFalse($result);
+    }
+
+    /**
      * testValidateHiddenMultipleModel method
      *
      * @return void

+ 21 - 0
tests/TestCase/Http/Middleware/CsrfProtectionMiddlewareTest.php

@@ -233,6 +233,27 @@ class CsrfProtectionMiddlewareTest extends TestCase
     }
 
     /**
+     * Test that request data works with the various http methods.
+     *
+     * @return void
+     */
+    public function testInvalidTokenNonStringData()
+    {
+        $this->expectException(\Cake\Http\Exception\InvalidCsrfTokenException::class);
+        $request = new ServerRequest([
+            'environment' => [
+                'REQUEST_METHOD' => 'POST',
+            ],
+            'post' => ['_csrfToken' => ['nope']],
+            'cookies' => ['csrfToken' => ['nope']],
+        ]);
+        $response = new Response();
+
+        $middleware = new CsrfProtectionMiddleware();
+        $middleware($request, $response, $this->_getNextClosure());
+    }
+
+    /**
      * Test that missing header and cookie fails
      *
      * @dataProvider httpMethodProvider

+ 81 - 32
tests/TestCase/Mailer/Transport/SmtpTransportTest.php

@@ -66,6 +66,19 @@ class SmtpTestTransport extends SmtpTransport
 class SmtpTransportTest extends TestCase
 {
     /**
+     * @var array
+     */
+    protected $credentials = [
+        'username' => 'mark',
+        'password' => 'story',
+    ];
+
+    /**
+     * @var string
+     */
+    protected $credentialsEncoded;
+
+    /**
      * Setup
      *
      * @return void
@@ -80,6 +93,8 @@ class SmtpTransportTest extends TestCase
         $this->SmtpTransport = new SmtpTestTransport();
         $this->SmtpTransport->setSocket($this->socket);
         $this->SmtpTransport->setConfig(['client' => 'localhost']);
+
+        $this->credentialsEncoded = base64_encode(chr(0) . 'mark' . chr(0) . 'story');
     }
 
     /**
@@ -154,13 +169,18 @@ class SmtpTransportTest extends TestCase
     {
         $this->expectException(\Cake\Network\Exception\SocketException::class);
         $this->expectExceptionMessage('SMTP authentication method not allowed, check if SMTP server requires TLS.');
-        $this->SmtpTransport->setConfig(['tls' => false, 'username' => 'user', 'password' => 'pass']);
+        $this->SmtpTransport->setConfig(['tls' => false] + $this->credentials);
+
         $this->socket->expects($this->any())->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 Accepted\r\n"));
-        $this->socket->expects($this->at(4))->method('write')->with("AUTH LOGIN\r\n");
-        $this->socket->expects($this->at(5))->method('read')
+
+        $this->socket->expects($this->at(4))->method('write')->with("AUTH PLAIN {$this->credentialsEncoded}\r\n");
+        $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized Authentication Type\r\n"));
+
+        $this->socket->expects($this->at(6))->method('write')->with("AUTH LOGIN\r\n");
+        $this->socket->expects($this->at(7))->method('read')
             ->will($this->returnValue("504 5.7.4 Unrecognized authentication type\r\n"));
         $this->SmtpTransport->connect();
     }
@@ -207,20 +227,31 @@ class SmtpTransportTest extends TestCase
         $this->assertContains('200 Not Accepted', $e->getPrevious()->getMessage());
     }
 
+    public function testAuthPlain()
+    {
+        $this->socket->expects($this->at(0))->method('write')->with("AUTH PLAIN {$this->credentialsEncoded}\r\n");
+        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("235 OK\r\n"));
+        $this->SmtpTransport->setConfig($this->credentials);
+        $this->SmtpTransport->auth();
+    }
+
     /**
      * testAuth method
      *
      * @return void
      */
-    public function testAuth()
+    public function testAuthLogin()
     {
-        $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
-        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("334 Login\r\n"));
-        $this->socket->expects($this->at(2))->method('write')->with("bWFyaw==\r\n");
-        $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("334 Pass\r\n"));
-        $this->socket->expects($this->at(4))->method('write')->with("c3Rvcnk=\r\n");
-        $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("235 OK\r\n"));
-        $this->SmtpTransport->setConfig(['username' => 'mark', 'password' => 'story']);
+        $this->socket->expects($this->at(0))->method('write')->with("AUTH PLAIN {$this->credentialsEncoded}\r\n");
+        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized Authentication Type\r\n"));
+
+        $this->socket->expects($this->at(2))->method('write')->with("AUTH LOGIN\r\n");
+        $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("334 Login\r\n"));
+        $this->socket->expects($this->at(4))->method('write')->with("bWFyaw==\r\n");
+        $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("334 Pass\r\n"));
+        $this->socket->expects($this->at(6))->method('write')->with("c3Rvcnk=\r\n");
+        $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("235 OK\r\n"));
+        $this->SmtpTransport->setConfig($this->credentials);
         $this->SmtpTransport->auth();
     }
 
@@ -233,10 +264,14 @@ class SmtpTransportTest extends TestCase
     {
         $this->expectException(\Cake\Network\Exception\SocketException::class);
         $this->expectExceptionMessage('AUTH command not recognized or not implemented, SMTP server may not require authentication.');
-        $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
-        $this->socket->expects($this->at(1))->method('read')
+
+        $this->socket->expects($this->at(0))->method('write')->with("AUTH PLAIN {$this->credentialsEncoded}\r\n");
+        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized Authentication Type\r\n"));
+
+        $this->socket->expects($this->at(2))->method('write')->with("AUTH LOGIN\r\n");
+        $this->socket->expects($this->at(3))->method('read')
             ->will($this->returnValue("500 5.3.3 Unrecognized command\r\n"));
-        $this->SmtpTransport->setConfig(['username' => 'mark', 'password' => 'story']);
+        $this->SmtpTransport->setConfig($this->credentials);
         $this->SmtpTransport->auth();
     }
 
@@ -249,10 +284,14 @@ class SmtpTransportTest extends TestCase
     {
         $this->expectException(\Cake\Network\Exception\SocketException::class);
         $this->expectExceptionMessage('AUTH command not recognized or not implemented, SMTP server may not require authentication.');
-        $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
-        $this->socket->expects($this->at(1))->method('read')
+
+        $this->socket->expects($this->at(0))->method('write')->with("AUTH PLAIN {$this->credentialsEncoded}\r\n");
+        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized Authentication Type\r\n"));
+
+        $this->socket->expects($this->at(2))->method('write')->with("AUTH LOGIN\r\n");
+        $this->socket->expects($this->at(3))->method('read')
             ->will($this->returnValue("502 5.3.3 Command not implemented\r\n"));
-        $this->SmtpTransport->setConfig(['username' => 'mark', 'password' => 'story']);
+        $this->SmtpTransport->setConfig($this->credentials);
         $this->SmtpTransport->auth();
     }
 
@@ -265,10 +304,14 @@ class SmtpTransportTest extends TestCase
     {
         $this->expectException(\Cake\Network\Exception\SocketException::class);
         $this->expectExceptionMessage('SMTP Error: 503 5.5.1 Already authenticated');
-        $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
-        $this->socket->expects($this->at(1))
+
+        $this->socket->expects($this->at(0))->method('write')->with("AUTH PLAIN {$this->credentialsEncoded}\r\n");
+        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized Authentication Type\r\n"));
+
+        $this->socket->expects($this->at(2))->method('write')->with("AUTH LOGIN\r\n");
+        $this->socket->expects($this->at(3))
             ->method('read')->will($this->returnValue("503 5.5.1 Already authenticated\r\n"));
-        $this->SmtpTransport->setConfig(['username' => 'mark', 'password' => 'story']);
+        $this->SmtpTransport->setConfig($this->credentials);
         $this->SmtpTransport->auth();
     }
 
@@ -279,12 +322,15 @@ class SmtpTransportTest extends TestCase
      */
     public function testAuthBadUsername()
     {
-        $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
-        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("334 Login\r\n"));
-        $this->socket->expects($this->at(2))->method('write')->with("bWFyaw==\r\n");
-        $this->socket->expects($this->at(3))->method('read')
+        $this->socket->expects($this->at(0))->method('write')->with("AUTH PLAIN {$this->credentialsEncoded}\r\n");
+        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized Authentication Type\r\n"));
+
+        $this->socket->expects($this->at(2))->method('write')->with("AUTH LOGIN\r\n");
+        $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("334 Login\r\n"));
+        $this->socket->expects($this->at(4))->method('write')->with("bWFyaw==\r\n");
+        $this->socket->expects($this->at(5))->method('read')
             ->will($this->returnValue("535 5.7.8 Authentication failed\r\n"));
-        $this->SmtpTransport->setConfig(['username' => 'mark', 'password' => 'story']);
+        $this->SmtpTransport->setConfig($this->credentials);
 
         $e = null;
         try {
@@ -305,13 +351,16 @@ class SmtpTransportTest extends TestCase
      */
     public function testAuthBadPassword()
     {
-        $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n");
-        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("334 Login\r\n"));
-        $this->socket->expects($this->at(2))->method('write')->with("bWFyaw==\r\n");
-        $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("334 Pass\r\n"));
-        $this->socket->expects($this->at(4))->method('write')->with("c3Rvcnk=\r\n");
-        $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("535 5.7.8 Authentication failed\r\n"));
-        $this->SmtpTransport->setConfig(['username' => 'mark', 'password' => 'story']);
+        $this->socket->expects($this->at(0))->method('write')->with("AUTH PLAIN {$this->credentialsEncoded}\r\n");
+        $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized Authentication Type\r\n"));
+
+        $this->socket->expects($this->at(2))->method('write')->with("AUTH LOGIN\r\n");
+        $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("334 Login\r\n"));
+        $this->socket->expects($this->at(4))->method('write')->with("bWFyaw==\r\n");
+        $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("334 Pass\r\n"));
+        $this->socket->expects($this->at(6))->method('write')->with("c3Rvcnk=\r\n");
+        $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("535 5.7.8 Authentication failed\r\n"));
+        $this->SmtpTransport->setConfig($this->credentials);
 
         $e = null;
         try {