[SECURITY] Disallow login with empty password 11/47611/2
authorHelmut Hummel <info@helhum.io>
Tue, 12 Apr 2016 09:11:37 +0000 (11:11 +0200)
committerOliver Hader <oliver.hader@typo3.org>
Tue, 12 Apr 2016 09:11:40 +0000 (11:11 +0200)
In case a backend or frontend user is stored in the database
with an empty string as password (not possible through backend UI),
it is possible to authenticate this user using an empty password
with the standard TYPO3 username/password authentication services.

By definition this should be prohibited.

Resolves: #75055
Releases: master, 7.6, 6.2
Security-Bulletins: TYPO3-CORE-SA-2016-009, 010, 011, 012
Change-Id: I4ca1b7d80c04de86d6ff1ef6e99a4a57b97ed948
Reviewed-on: https://review.typo3.org/47611
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
typo3/sysext/core/Classes/Authentication/AbstractUserAuthentication.php
typo3/sysext/saltedpasswords/Classes/SaltedPasswordService.php
typo3/sysext/sv/Classes/AuthenticationService.php
typo3/sysext/sv/ext_localconf.php

index 7b53083..c5a1017 100644 (file)
@@ -588,7 +588,7 @@ abstract class AbstractUserAuthentication
             GeneralUtility::devLog('Login data: ' . GeneralUtility::arrayToLogString($loginData), AbstractUserAuthentication::class);
         }
         // Active logout (eg. with "logout" button)
-        if ($loginData['status'] == 'logout') {
+        if ($loginData['status'] === 'logout') {
             if ($this->writeStdLog) {
                 // $type,$action,$error,$details_nr,$details,$data,$tablename,$recuid,$recpid
                 $this->writelog(255, 2, 0, 2, 'User %s logged out', array($this->user['username']), '', 0, 0);
@@ -600,7 +600,7 @@ abstract class AbstractUserAuthentication
             $this->logoff();
         }
         // Active login (eg. with login form)
-        if ($loginData['status'] == 'login') {
+        if ($loginData['status'] === 'login') {
             $activeLogin = true;
             if ($this->writeDevLog) {
                 GeneralUtility::devLog('Active login (eg. with login form)', AbstractUserAuthentication::class);
@@ -1400,7 +1400,7 @@ abstract class AbstractUserAuthentication
      */
     public function compareUident($user, $loginData, $passwordCompareStrategy = '')
     {
-        return (string)$loginData['uident_text'] === (string)$user[$this->userident_column];
+        return (string)$loginData['uident_text'] !== '' && (string)$loginData['uident_text'] === (string)$user[$this->userident_column];
     }
 
     /**
index 9accafe..5b0220a 100644 (file)
@@ -147,7 +147,7 @@ class SaltedPasswordService extends \TYPO3\CMS\Sv\AbstractAuthenticationService
                     $this->authenticationFailed = true;
                 }
             } else {
-                $validPasswd = (string)$password === (string)$user['password'];
+                $validPasswd = (string)$password !== '' && (string)$password === (string)$user['password'];
             }
             // Should we store the new format value in DB?
             if ($validPasswd && (int)$this->extConf['updatePasswd']) {
@@ -173,11 +173,10 @@ class SaltedPasswordService extends \TYPO3\CMS\Sv\AbstractAuthenticationService
     public function authUser(array $user)
     {
         $OK = 100;
-        $validPasswd = false;
-        if ($this->login['uident'] && $this->login['uname']) {
-            if (!empty($this->login['uident_text'])) {
-                $validPasswd = $this->compareUident($user, $this->login);
-            }
+        // The salted password service can only work correctly, if a non empty username along with a non empty password is provided.
+        // Otherwise a different service is allowed to check for other login credentials
+        if ((string)$this->login['uident_text'] !== '' && (string)$this->login['uname'] !== '') {
+            $validPasswd = $this->compareUident($user, $this->login);
             if (!$validPasswd) {
                 // Failed login attempt (wrong password)
                 $errorMessage = 'Login-attempt from %s (%s), username \'%s\', password not accepted!';
index 2275e7f..c3cad68 100644 (file)
@@ -49,7 +49,7 @@ class AuthenticationService extends AbstractAuthenticationService
         if ($this->login['status'] !== 'login') {
             return false;
         }
-        if (!$this->login['uident']) {
+        if ((string)$this->login['uident_text'] === '') {
             // Failed Login attempt (no password given)
             $this->writelog(255, 3, 3, 2, 'Login-attempt from %s (%s) for username \'%s\' with an empty password!', array(
                 $this->authInfo['REMOTE_ADDR'], $this->authInfo['REMOTE_HOST'], $this->login['uname']
@@ -88,7 +88,9 @@ class AuthenticationService extends AbstractAuthenticationService
     public function authUser(array $user)
     {
         $OK = 100;
-        if ($this->login['uident'] && $this->login['uname']) {
+        // This authentication service can only work correctly, if a non empty username along with a non empty password is provided.
+        // Otherwise a different service is allowed to check for other login credentials
+        if ((string)$this->login['uident_text'] !== '' && (string)$this->login['uname'] !== '') {
             // Checking password match for user:
             $OK = $this->compareUident($user, $this->login);
             if (!$OK) {
index 02c5c01..26c5fba 100644 (file)
@@ -9,7 +9,7 @@ defined('TYPO3_MODE') or die();
     array(
         'title' => 'User authentication',
         'description' => 'Authentication with username/password.',
-        'subtype' => 'getUserBE,authUserBE,getUserFE,authUserFE,getGroupsFE,processLoginDataBE,processLoginDataFE',
+        'subtype' => 'getUserBE,getUserFE,authUserFE,getGroupsFE,processLoginDataBE,processLoginDataFE',
         'available' => true,
         'priority' => 50,
         'quality' => 50,