Blocked user and forget password button

I stumbled on a possible hole with blocked users and the password retrieval feature. It seems that if you’re a blocked user and if you have access to the "forgot password" page, you can essentially just reset your password and unblock yourself.

I’m using the downloadable 1.3.1 beta 1 from the repo so I’m not sure if this has been fixed yet in the latest development version. For those looking for a fix, here’s my patch to the webloginpe.class.php:


function ActivateUser()
{
global $modx;

$web_users = $modx->getFullTableName(’web_users’);
$web_user_attributes = $modx->getFullTableName(’web_user_attributes’);

$userid = $modx->db->escape($_REQUEST[’userid’]);
$activationKey = $modx->db->escape($_REQUEST[’activationkey’]);
$passwordKey = $modx->db->escape($_POST[’activationpassword’]);
$newPassword = $modx->db->escape($_POST[’newpassword’]);
$newPasswordConfirm = $modx->db->escape($_POST[’newpassword_confirm’]); // pixelchutes 1:55 AM 9/19/2007

$findUser = "SELECT * FROM ".$web_users." WHERE id=’".$userid."’";
$userInfo = $modx->db->query($findUser);
$checkIfBlocked = "SELECT blocked FROM ".modx_web_user_attributes." WHERE internalKey=’".$userid."’";
$userBlockedInfo = $modx->db->query($checkIfBlocked);
$limit = $modx->recordCount($userInfo);

if ($limit !==1)
{
return $this->FormatMessage($this->LanguageArray[15]);
}

$this->User = $modx->db->getRow($userInfo);
list($cachePassword, $cacheKey) = explode("|",$this->User[’cachepwd’]);

if (($passwordKey !== $cachePassword) || ($activationKey !== $cacheKey))
{
return $this->FormatMessage($this->LanguageArray[16]);
}

if (!empty($newPassword) && isset($newPassword) && isset($newPasswordConfirm))
{
if ($newPassword === $newPasswordConfirm)
{
if (md5($newPassword) === md5($modx->db->escape($newPassword)))
{
if (strlen($newPassword) > 5)
{
$passwordElement = "UPDATE ".$web_users." SET `password`=’".md5($modx->db->escape($newPassword))."’, cachepwd=’’ WHERE `id`=’".$this->User[’id’]."’";
$saveMyPassword = $modx->db->query($passwordElement);

//CHECK IF BLOCKED
if ($modx->db->getValue($userBlockedInfo) == 0) {

$blocks = "UPDATE ".$web_user_attributes." SET `blocked`=’0’, `blockeduntil`=’0’ WHERE `internalKey`=’".$this->User[’id’]."’";

} else {

$blocks = "UPDATE ".$web_user_attributes." SET `blocked`=’1’, `blockeduntil`=’0’ WHERE `internalKey`=’".$this->User[’id’]."’";

}

$unblockUser = $modx->db->query($blocks);


....



Text in red is what I’ve added or changed.

If someone can incorporate this into the latest build in a more elegant manner (if it hasn’t been fixed yet), that’d be great - I’m still learning how the innards of the MODx system works.