Skip to content

Commit

Permalink
[SS-2016-011] ChangePasswordForm does not check $member->canLogin bef…
Browse files Browse the repository at this point in the history
…ore login
  • Loading branch information
dhensby authored and Damian Mooyman committed Aug 15, 2016
1 parent 08384bb commit 782c18f
Showing 1 changed file with 13 additions and 10 deletions.
23 changes: 13 additions & 10 deletions security/ChangePasswordForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* @subpackage security
*/
class ChangePasswordForm extends Form {

/**
* Constructor
*
Expand All @@ -28,7 +28,7 @@ public function __construct($controller, $name, $fields = null, $actions = null)

if(!$fields) {
$fields = new FieldList();

// Security/changepassword?h=XXX redirects to Security/changepassword
// without GET parameter to avoid potential HTTP referer leakage.
// In this case, a user is not logged in, and no 'old password' should be necessary.
Expand Down Expand Up @@ -65,7 +65,7 @@ public function doChangePassword(array $data) {
if(empty($data['OldPassword']) || !$member->checkPassword($data['OldPassword'])->valid()) {
$this->clearMessage();
$this->sessionMessage(
_t('Member.ERRORPASSWORDNOTMATCH', "Your current password does not match, please try again"),
_t('Member.ERRORPASSWORDNOTMATCH', "Your current password does not match, please try again"),
"bad"
);
// redirect back to the form, instead of using redirectBack() which could send the user elsewhere.
Expand Down Expand Up @@ -98,18 +98,21 @@ public function doChangePassword(array $data) {
else if($data['NewPassword1'] == $data['NewPassword2']) {
$isValid = $member->changePassword($data['NewPassword1']);
if($isValid->valid()) {
$member->logIn();

// TODO Add confirmation message to login redirect
Session::clear('AutoLoginHash');

// Clear locked out status
$member->LockedOutUntil = null;
$member->FailedLoginCount = null;
$member->write();

if ($member->canLogIn()->valid()) {
$member->logIn();
}

// TODO Add confirmation message to login redirect
Session::clear('AutoLoginHash');

if (!empty($_REQUEST['BackURL'])
// absolute redirection URLs may cause spoofing
// absolute redirection URLs may cause spoofing
&& Director::is_site_url($_REQUEST['BackURL'])
) {
$url = Director::absoluteURL($_REQUEST['BackURL']);
Expand All @@ -127,10 +130,10 @@ public function doChangePassword(array $data) {
$this->clearMessage();
$this->sessionMessage(
_t(
'Member.INVALIDNEWPASSWORD',
'Member.INVALIDNEWPASSWORD',
"We couldn't accept that password: {password}",
array('password' => nl2br("\n".Convert::raw2xml($isValid->starredList())))
),
),
"bad",
false
);
Expand Down

0 comments on commit 782c18f

Please sign in to comment.