CVE-2019-10641

@@ -1,6 +1,13 @@
 Contao Open Source CMS changelog
 ================================
 
+Version 3.5.39 (2019-04-XX)
+---------------------------
+
+### Fixed
+Invalidate the user sessions if a password changes (see CVE-2019-10641).
+
+
 Version 3.5.38 (2018-12-21)
 ---------------------------
 

@@ -608,6 +608,10 @@ public function setNewPassword($strPassword, $user)
 			}
 		}
 
+		// Invalidate the user sessions if the password changes
+		$this->Database->prepare("DELETE FROM tl_session WHERE name='FE_USER_AUTH' AND pid=? AND sessionID!=?")
+					   ->execute($user->id, session_id());
+
 		return $strPassword;
 	}
 

@@ -242,6 +242,10 @@
 			'exclude'                 => true,
 			'inputType'               => 'password',
 			'eval'                    => array('mandatory'=>true, 'preserveTags'=>true, 'minlength'=>Config::get('minPasswordLength')),
+			'save_callback' => array
+			(
+				array('tl_user', 'invalidateSessions')
+			),
 			'sql'                     => "varchar(128) NOT NULL default ''"
 		),
 		'pwChange' => array
@@ -732,6 +736,26 @@ public function checkAdminStatus($varValue, DataContainer $dc)
 	}
 
 
+	/**
+	 * Invalidate the user sessions if the password changes
+	 *
+	 * The password widget only triggers the save_callback if the password has actually
+	 * changed, therefore we do not need to check the active record here.
+	 *
+	 * @param mixed         $varValue
+	 * @param DataContainer $dc
+	 *
+	 * @return mixed
+	 */
+	public function invalidateSessions($varValue, DataContainer $dc)
+	{
+		$this->Database->prepare("DELETE FROM tl_session WHERE name='BE_USER_AUTH' AND pid=? AND sessionID!=?")
+					   ->execute($dc->id, session_id());
+
+		return $varValue;
+	}
+
+
 	/**
 	 * Prevent administrators from disabling their own account
 	 *

@@ -186,6 +186,10 @@ protected function compile()
 				}
 			}
 
+			// Invalidate the user sessions if the password changes
+			$this->Database->prepare("DELETE FROM tl_session WHERE name='FE_USER_AUTH' AND pid=? AND sessionID!=?")
+						   ->execute($objMember->id, session_id());
+
 			// Check whether there is a jumpTo page
 			if (($objJumpTo = $this->objModel->getRelated('jumpTo')) !== null)
 			{

@@ -254,6 +254,10 @@ protected function setNewPassword()
 					}
 				}
 
+				// Invalidate the user sessions if the password changes
+				$this->Database->prepare("DELETE FROM tl_session WHERE name='FE_USER_AUTH' AND pid=? AND sessionID!=?")
+							   ->execute($objMember->id, session_id());
+
 				// Redirect to the jumpTo page
 				if (($objTarget = $this->objModel->getRelated('reg_jumpTo')) !== null)
 				{

@@ -2,6 +2,7 @@
 
 ## DEV
 
+ * Invalidate the user sessions if a password changes (see CVE-2019-10641).
  * Correctly check if a file or folder is excluded from synchronization (see 410).
 
 ## 4.7.2 (2019-03-25)

@@ -24,7 +24,8 @@
 		),
 		'onsubmit_callback' => array
 		(
-			array('tl_user', 'storeDateAdded')
+			array('tl_user', 'storeDateAdded'),
+			array('tl_user', 'updateCurrentUser')
 		),
 		'sql' => array
 		(
@@ -871,6 +872,20 @@ public function storeDateAdded(Contao\DataContainer $dc)
 					   ->execute($time, $dc->id);
 	}
 
+	/**
+	 * Update the current user if something changes, otherwise they would be
+	 * logged out automatically
+	 *
+	 * @param Contao\DataContainer $dc
+	 */
+	public function updateCurrentUser(Contao\DataContainer $dc)
+	{
+		if ($this->User->id == $dc->id)
+		{
+			$this->User->findBy('id', $this->User->id);
+		}
+	}
+
 	/**
 	 * Return the "toggle visibility" button
 	 *

@@ -425,7 +425,7 @@ public function isMemberOf($id)
 			return false;
 		}
 
-		$groups = StringUtil::deserialize($this->arrData['groups']);
+		$groups = StringUtil::deserialize($this->groups);
 
 		// No groups assigned
 		if (empty($groups) || !\is_array($groups))
@@ -519,15 +519,15 @@ public static function loadUserByUsername($username)
 	 */
 	public function getUsername()
 	{
-		return $this->arrData['username'];
+		return $this->username;
 	}
 
 	/**
 	 * {@inheritdoc}
 	 */
 	public function setUsername($username)
 	{
-		$this->arrData['username'] = $username;
+		$this->username = $username;
 
 		return $this;
 	}
@@ -537,15 +537,15 @@ public function setUsername($username)
 	 */
 	public function getPassword()
 	{
-		return $this->arrData['password'];
+		return $this->password;
 	}
 
 	/**
 	 * {@inheritdoc}
 	 */
 	public function setPassword($password)
 	{
-		$this->arrData['password'] = $password;
+		$this->password = $password;
 
 		return $this;
 	}
@@ -573,15 +573,33 @@ public function setSalt($salt)
 	 */
 	public function serialize()
 	{
-		return serialize(array($this->id, $this->username, $this->disable, $this->admin, $this->groups));
+		$data = array
+		(
+			'id' => $this->id,
+			'username' => $this->username,
+			'password' => $this->password,
+			'admin' => $this->admin,
+			'disable' => $this->disable,
+			'start' => $this->start,
+			'stop' => $this->stop
+		);
+
+		return serialize($data);
 	}
 
 	/**
 	 * {@inheritdoc}
 	 */
 	public function unserialize($serialized)
 	{
-		list($this->id, $this->username, $this->disable, $this->admin, $this->groups) = unserialize($serialized, array('allowed_classes'=>false));
+		$data = unserialize($serialized, array('allowed_classes'=>false));
+
+		if (array_keys($data) != array('id', 'username', 'password', 'admin', 'disable', 'start', 'stop'))
+		{
+			return;
+		}
+
+		list($this->id, $this->username, $this->password, $this->admin, $this->disable, $this->start, $this->stop) = array_values($data);
 	}
 
 	/**
@@ -604,12 +622,12 @@ public function isEqualTo(UserInterface $user)
 			return false;
 		}
 
-		if ((bool) $this->admin !== (bool) $user->admin)
+		if ($this->password !== $user->password)
 		{
 			return false;
 		}
 
-		if ($this->groups !== $user->groups)
+		if ((bool) $this->admin !== (bool) $user->admin)
 		{
 			return false;
 		}
@@ -619,6 +637,18 @@ public function isEqualTo(UserInterface $user)
 			return false;
 		}
 
+		$time = Date::floorToMinute();
+
+		if ($this->start !== '' && $this->start > $time)
+		{
+			return false;
+		}
+
+		if ($this->stop !== '' && $this->stop <= ($time + 60))
+		{
+			return false;
+		}
+
 		return true;
 	}
 

@@ -185,6 +185,9 @@ protected function compile()
 				}
 			}
 
+			// Update the current user so they are not logged out automatically
+			$this->User->findBy('id', $objMember->id);
+
 			// Check whether there is a jumpTo page
 			if (($objJumpTo = $this->objModel->getRelated('jumpTo')) instanceof PageModel)
 			{

@@ -1,5 +1,9 @@
 # Change log
 
+## DEV
+
+ * Invalidate the user sessions if a password changes (see CVE-2019-10641).
+
 ## 4.4.36 (2019-03-25)
 
  * Make custom layout section titles and IDs mandatory (see #341).

@@ -607,6 +607,10 @@ public function setNewPassword($strPassword, $user)
 			}
 		}
 
+		// Invalidate the user sessions if the password changes
+		$this->Database->prepare("DELETE FROM tl_session WHERE name='FE_USER_AUTH' AND pid=? AND sessionID!=?")
+					   ->execute($user->id, session_id());
+
 		return $strPassword;
 	}
 

@@ -254,6 +254,10 @@
 			'exclude'                 => true,
 			'inputType'               => 'password',
 			'eval'                    => array('mandatory'=>true, 'preserveTags'=>true, 'minlength'=>Config::get('minPasswordLength')),
+			'save_callback' => array
+			(
+				array('tl_user', 'invalidateSessions')
+			),
 			'sql'                     => "varchar(255) NOT NULL default ''"
 		),
 		'pwChange' => array
@@ -742,6 +746,25 @@ public function getModules()
 		return $arrModules;
 	}
 
+	/**
+	 * Invalidate the user sessions if the password changes
+	 *
+	 * The password widget only triggers the save_callback if the password has actually
+	 * changed, therefore we do not need to check the active record here.
+	 *
+	 * @param mixed         $varValue
+	 * @param DataContainer $dc
+	 *
+	 * @return mixed
+	 */
+	public function invalidateSessions($varValue, DataContainer $dc)
+	{
+		$this->Database->prepare("DELETE FROM tl_session WHERE name='BE_USER_AUTH' AND pid=? AND sessionID!=?")
+					   ->execute($dc->id, session_id());
+
+		return $varValue;
+	}
+
 	/**
 	 * Prevent administrators from downgrading their own account
 	 *

@@ -187,6 +187,10 @@ protected function compile()
 				}
 			}
 
+			// Invalidate the user sessions if the password changes
+			$this->Database->prepare("DELETE FROM tl_session WHERE name='FE_USER_AUTH' AND pid=? AND sessionID!=?")
+						   ->execute($objMember->id, session_id());
+
 			// Check whether there is a jumpTo page
 			if (($objJumpTo = $this->objModel->getRelated('jumpTo')) instanceof PageModel)
 			{

@@ -254,6 +254,10 @@ protected function setNewPassword()
 					}
 				}
 
+				// Invalidate the user sessions if the password changes
+				$this->Database->prepare("DELETE FROM tl_session WHERE name='FE_USER_AUTH' AND pid=? AND sessionID!=?")
+							   ->execute($objMember->id, session_id());
+
 				// Redirect to the jumpTo page
 				if (($objTarget = $this->objModel->getRelated('reg_jumpTo')) instanceof PageModel)
 				{