From 447ce0f84f880c2bc969a89e4be528c53caeabe0 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 9 May 2017 21:24:15 +0100 Subject: [PATCH 1/6] [SS-2017-002] FIX Lock out users who dont exist in the DB --- security/Member.php | 33 +++++++++++++++-- security/MemberAuthenticator.php | 10 +++++- tests/security/MemberAuthenticatorTest.php | 41 +++++++++++++++++++++- tests/security/SecurityTest.php | 18 +++++----- 4 files changed, 88 insertions(+), 14 deletions(-) diff --git a/security/Member.php b/security/Member.php index 692ff61a5..2648c800d 100644 --- a/security/Member.php +++ b/security/Member.php @@ -398,7 +398,35 @@ class Member extends DataObject implements TemplateGlobalProvider { * Returns true if this user is locked out */ public function isLockedOut() { - return $this->LockedOutUntil && SS_Datetime::now()->Format('U') < strtotime($this->LockedOutUntil); + global $debug; + if ($this->LockedOutUntil && $this->dbObject('LockedOutUntil')->InFuture()) { + return true; + } + + if ($this->config()->lock_out_after_incorrect_logins <= 0) { + return false; + } + + $attempts = LoginAttempt::get()->filter($filter = array( + 'Email' => $this->{static::config()->unique_identifier_field}, + ))->sort('Created', 'DESC')->limit($this->config()->lock_out_after_incorrect_logins); + + if ($attempts->count() < $this->config()->lock_out_after_incorrect_logins) { + return false; + } + + foreach ($attempts as $attempt) { + if ($attempt->Status === 'Success') { + return false; + } + } + + $lockedOutUntil = $attempts->first()->dbObject('Created')->Format('U') + ($this->config()->lock_out_delay_mins * 60); + if (SS_Datetime::now()->Format('U') < $lockedOutUntil) { + return true; + } + + return false; } /** @@ -1660,7 +1688,7 @@ class Member extends DataObject implements TemplateGlobalProvider { public function registerFailedLogin() { if(self::config()->lock_out_after_incorrect_logins) { // Keep a tally of the number of failed log-ins so that we can lock people out - $this->FailedLoginCount = $this->FailedLoginCount + 1; + ++$this->FailedLoginCount; if($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) { $lockoutMins = self::config()->lock_out_delay_mins; @@ -1679,6 +1707,7 @@ class Member extends DataObject implements TemplateGlobalProvider { if(self::config()->lock_out_after_incorrect_logins) { // Forgive all past login failures $this->FailedLoginCount = 0; + $this->LockedOutUntil = null; $this->write(); } } diff --git a/security/MemberAuthenticator.php b/security/MemberAuthenticator.php index 4a1635ab6..01586ce65 100644 --- a/security/MemberAuthenticator.php +++ b/security/MemberAuthenticator.php @@ -70,6 +70,14 @@ class MemberAuthenticator extends Authenticator { if($member && !$asDefaultAdmin) { $result = $member->checkPassword($data['Password']); $success = $result->valid(); + } elseif (!$asDefaultAdmin) { + // spoof a login attempt + $member = Member::create(); + $member->Email = $email; + $member->{Member::config()->unique_identifier_field} = $data['Password'] . '-wrong'; + $member->PasswordEncryption = 'none'; + $result = $member->checkPassword($data['Password']); + $member = null; } else { $result = new ValidationResult(false, _t('Member.ERRORWRONGCRED')); } @@ -94,7 +102,7 @@ class MemberAuthenticator extends Authenticator { * @param bool $success */ protected static function record_login_attempt($data, $member, $success) { - if(!Security::config()->login_recording) return; + if(!Security::config()->login_recording && !Member::config()->lock_out_after_incorrect_logins) return; // Check email is valid $email = isset($data['Email']) ? $data['Email'] : null; diff --git a/tests/security/MemberAuthenticatorTest.php b/tests/security/MemberAuthenticatorTest.php index d5b9ac8d2..8606e6f58 100644 --- a/tests/security/MemberAuthenticatorTest.php +++ b/tests/security/MemberAuthenticatorTest.php @@ -180,6 +180,45 @@ class MemberAuthenticatorTest extends SapphireTest { ), $form); $this->assertTrue(Member::default_admin()->isLockedOut()); - $this->assertEquals(Member::default_admin()->LockedOutUntil, '2016-04-18 00:10:00'); + $this->assertEquals('2016-04-18 00:10:00', Member::default_admin()->LockedOutUntil); + } + + public function testNonExistantMemberGetsLoginAttemptRecorded() + { + Config::inst()->update('Member', 'lock_out_after_incorrect_logins', 1); + $email = 'notreal@example.com'; + $this->assertFalse(Member::get()->filter(array('Email' => $email))->exists()); + $this->assertCount(0, LoginAttempt::get()); + $response = MemberAuthenticator::authenticate(array( + 'Email' => $email, + 'Password' => 'password', + )); + $this->assertNull($response); + $this->assertCount(1, LoginAttempt::get()); + $attempt = LoginAttempt::get()->first(); + $this->assertEquals($email, $attempt->Email); + $this->assertEquals('Failure', $attempt->Status); + + } + + public function testNonExistantMemberGetsLockedOut() + { + Config::inst()->update('Member', 'lock_out_after_incorrect_logins', 1); + Config::inst()->update('Member', 'lock_out_delay_mins', 10); + $email = 'notreal@example.com'; + + $this->assertFalse(Member::get()->filter(array('Email' => $email))->exists()); + + $response = MemberAuthenticator::authenticate(array( + 'Email' => $email, + 'Password' => 'password' + )); + + $this->assertNull($response); + $member = new Member(); + $member->Email = $email; + + $this->assertTrue($member->isLockedOut()); + $this->assertFalse($member->canLogIn()->valid()); } } diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index 45463d6fe..4edc90a7c 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -402,6 +402,7 @@ class SecurityTest extends FunctionalTest { public function testRepeatedLoginAttemptsLockingPeopleOut() { $local = i18n::get_locale(); i18n::set_locale('en_US'); + SS_Datetime::set_mock_now(DBField::create_field('SS_Datetime', '2017-05-22 00:00:00')); Member::config()->lock_out_after_incorrect_logins = 5; Member::config()->lock_out_delay_mins = 15; @@ -418,10 +419,9 @@ class SecurityTest extends FunctionalTest { ); $this->assertContains($this->loginErrorMessage(), Convert::raw2xml(_t('Member.ERRORWRONGCRED'))); } else { - // Fuzzy matching for time to avoid side effects from slow running tests - $this->assertGreaterThan( - time() + 14*60, - strtotime($member->LockedOutUntil), + $this->assertEquals( + SS_Datetime::now()->Format('U') + (15 * 60), + $member->dbObject('LockedOutUntil')->Format('U'), 'User has a lockout time set after too many failed attempts' ); } @@ -444,14 +444,12 @@ class SecurityTest extends FunctionalTest { 'The user can\'t log in after being locked out, even with the right password' ); - // (We fake this by re-setting LockedOutUntil) - $member = DataObject::get_by_id("Member", $this->idFromFixture('Member', 'test')); - $member->LockedOutUntil = date('Y-m-d H:i:s', time() - 30); - $member->write(); + // Move into the future so we can login again + SS_Datetime::set_mock_now(DBField::create_field('SS_Datetime', '2017-06-22 00:00:00')); $this->doTestLoginForm('testuser@example.com' , '1nitialPassword'); $this->assertEquals( - $this->session()->inst_get('loggedInAs'), $member->ID, + $this->session()->inst_get('loggedInAs'), 'After lockout expires, the user can login again' ); @@ -471,8 +469,8 @@ class SecurityTest extends FunctionalTest { $this->doTestLoginForm('testuser@example.com' , '1nitialPassword'); $this->assertEquals( - $this->session()->inst_get('loggedInAs'), $member->ID, + $this->session()->inst_get('loggedInAs'), 'The user can login successfully after lockout expires, if staying below the threshold' ); From eeb549faf37df5aee1840423d5aefc25bb51c612 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Sun, 28 May 2017 21:34:38 +0000 Subject: [PATCH 2/6] Added 3.4.6-rc1 changelog --- docs/en/04_Changelogs/rc/3.4.6-rc1.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 docs/en/04_Changelogs/rc/3.4.6-rc1.md diff --git a/docs/en/04_Changelogs/rc/3.4.6-rc1.md b/docs/en/04_Changelogs/rc/3.4.6-rc1.md new file mode 100644 index 000000000..0bea252f0 --- /dev/null +++ b/docs/en/04_Changelogs/rc/3.4.6-rc1.md @@ -0,0 +1,18 @@ +# 3.4.6-rc1 + + + +## Change Log + +### Security + + * 2017-05-24 [41270fc](https://github.com/silverstripe/silverstripe-cms/commit/41270fcf9980c4be2529d2750c717675548eb617) Only allow HTTP(S) links for external redirector pages (Daniel Hensby) - See [ss-2017-003](http://www.silverstripe.org/download/security-releases/ss-2017-003) + * 2017-05-09 [447ce0f](https://github.com/silverstripe/silverstripe-framework/commit/447ce0f84f880c2bc969a89e4be528c53caeabe0) Lock out users who dont exist in the DB (Daniel Hensby) - See [ss-2017-002](http://www.silverstripe.org/download/security-releases/ss-2017-002) + * 2017-05-09 [61cf72c](https://github.com/silverstripe/silverstripe-cms/commit/61cf72c08dafddef416d73f943ccd45e70c5d43d) Unescaped fields in CMSPageHistroyController::compare() (Daniel Hensby) - See [ss-2017-004](http://www.silverstripe.org/download/security-releases/ss-2017-004) + +### Bugfixes + + * 2017-05-03 [2d138b0](https://github.com/silverstripe/silverstripe-framework/commit/2d138b0ef06bd93958cc0678a0afa95560648fb9) class name reference consistency (Gregory Smirnov) + * 2017-04-24 [1d36f35](https://github.com/silverstripe/silverstripe-framework/commit/1d36f354e8349616c7b39fcade859fbcf0f9c362) Create Image_Cached with Injector. (Gregory Smirnov) + * 2017-02-15 [3072591](https://github.com/silverstripe/silverstripe-framework/commit/30725916dbb0ffc66b77f26c069a86581636ae55) Array to string conversion message after CSV export (#6622) (Juan van den Anker) + * 2017-02-14 [7122e1f](https://github.com/silverstripe/silverstripe-framework/commit/7122e1fde79bdb9aad3c8714a6ce02b7ecedd735) Comments ignored by classmanifest (#6619) (Daniel Hensby) From 16a74bc8a9fdee7cfb4f6f24493c271f90a76341 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Sun, 28 May 2017 23:59:00 +0100 Subject: [PATCH 3/6] FIX DataDifferencer needs to expliclty cast HTMLText values --- model/DataDifferencer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/DataDifferencer.php b/model/DataDifferencer.php index 0c33b1206..0cd573259 100644 --- a/model/DataDifferencer.php +++ b/model/DataDifferencer.php @@ -103,7 +103,7 @@ class DataDifferencer extends ViewableData { // Show changes between the two, if any exist if($fromValue != $toValue) { - $diffed->setField($field, Diff::compareHTML($fromValue, $toValue)); + $diffed->setField($field, DBField::create_field('HTMLText', Diff::compareHTML($fromValue, $toValue))); } } From b5ad4bdcc622cbe7aed496e7ad3d92708624b80e Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Sun, 28 May 2017 23:49:04 +0000 Subject: [PATCH 4/6] Added 3.4.6-rc2 changelog --- docs/en/04_Changelogs/rc/3.4.6-rc2.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 docs/en/04_Changelogs/rc/3.4.6-rc2.md diff --git a/docs/en/04_Changelogs/rc/3.4.6-rc2.md b/docs/en/04_Changelogs/rc/3.4.6-rc2.md new file mode 100644 index 000000000..d3a388e73 --- /dev/null +++ b/docs/en/04_Changelogs/rc/3.4.6-rc2.md @@ -0,0 +1,9 @@ +# 3.4.6-rc2 + + + +## Change Log + +### Bugfixes + + * 2017-05-28 [16a74bc](https://github.com/silverstripe/silverstripe-framework/commit/16a74bc8a9fdee7cfb4f6f24493c271f90a76341) DataDifferencer needs to expliclty cast HTMLText values (Daniel Hensby) From 9a38bedd18187b820bfd46cb26e34ce44ffc3037 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Mon, 29 May 2017 00:08:27 +0000 Subject: [PATCH 5/6] Added 3.5.4-rc1 changelog --- docs/en/04_Changelogs/rc/3.5.4-rc1.md | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/en/04_Changelogs/rc/3.5.4-rc1.md diff --git a/docs/en/04_Changelogs/rc/3.5.4-rc1.md b/docs/en/04_Changelogs/rc/3.5.4-rc1.md new file mode 100644 index 000000000..eae808d35 --- /dev/null +++ b/docs/en/04_Changelogs/rc/3.5.4-rc1.md @@ -0,0 +1,28 @@ +# 3.5.4-rc1 + + + +## Change Log + +### Security + + * 2017-05-24 [41270fc](https://github.com/silverstripe/silverstripe-cms/commit/41270fcf9980c4be2529d2750c717675548eb617) Only allow HTTP(S) links for external redirector pages (Daniel Hensby) - See [ss-2017-003](http://www.silverstripe.org/download/security-releases/ss-2017-003) + * 2017-05-09 [447ce0f](https://github.com/silverstripe/silverstripe-framework/commit/447ce0f84f880c2bc969a89e4be528c53caeabe0) Lock out users who dont exist in the DB (Daniel Hensby) - See [ss-2017-002](http://www.silverstripe.org/download/security-releases/ss-2017-002) + * 2017-05-09 [61cf72c](https://github.com/silverstripe/silverstripe-cms/commit/61cf72c08dafddef416d73f943ccd45e70c5d43d) Unescaped fields in CMSPageHistroyController::compare() (Daniel Hensby) - See [ss-2017-004](http://www.silverstripe.org/download/security-releases/ss-2017-004) + +### Bugfixes + + * 2017-05-28 [16a74bc](https://github.com/silverstripe/silverstripe-framework/commit/16a74bc8a9fdee7cfb4f6f24493c271f90a76341) DataDifferencer needs to expliclty cast HTMLText values (Daniel Hensby) + * 2017-05-08 [1454072](https://github.com/silverstripe/silverstripe-cms/commit/14540729caa30dd2e782e4fd52afe518dc156ed8) Use framework 3.5 to test cms 3.5 (Sam Minnee) + * 2017-05-03 [2d138b0](https://github.com/silverstripe/silverstripe-framework/commit/2d138b0ef06bd93958cc0678a0afa95560648fb9) class name reference consistency (Gregory Smirnov) + * 2017-05-02 [2187c16](https://github.com/silverstripe/silverstripe-framework/commit/2187c160b936620621fe746a1ffe36af568b21ff) ing pagination api doc typo (3Dgoo) + * 2017-04-28 [a511e35](https://github.com/silverstripe/silverstripe-framework/commit/a511e3511cace405dab7589a3406a0858cb6edf2) #6855: Mangled JS in Requirements, escaping replacement values prior to passing to preg_replace(). (Patrick Nelson) + * 2017-04-24 [1d36f35](https://github.com/silverstripe/silverstripe-framework/commit/1d36f354e8349616c7b39fcade859fbcf0f9c362) Create Image_Cached with Injector. (Gregory Smirnov) + * 2017-04-07 [55eb7eb](https://github.com/silverstripe/silverstripe-framework/commit/55eb7ebdcc9ba767f978dff510614bbd2e0c309d) Do not insert requirements more than once in includeInHTML (Robbie Averill) + * 2017-04-05 [a7920b1](https://github.com/silverstripe/silverstripe-framework/commit/a7920b1f9866b6eb5f4bad9de84eef84b88673ad) regression from #6668 - ModelAdmin form widths (Loz Calver) + * 2017-04-05 [197bc53](https://github.com/silverstripe/silverstripe-framework/commit/197bc53c4963898d2c10621ca6d6031fdb14fe85) Add transparency percent argument to Image::generatePad to ensure transparency works from ::Pad (Robbie Averill) + * 2017-02-15 [3072591](https://github.com/silverstripe/silverstripe-framework/commit/30725916dbb0ffc66b77f26c069a86581636ae55) Array to string conversion message after CSV export (#6622) (Juan van den Anker) + * 2017-02-14 [7122e1f](https://github.com/silverstripe/silverstripe-framework/commit/7122e1fde79bdb9aad3c8714a6ce02b7ecedd735) Comments ignored by classmanifest (#6619) (Daniel Hensby) + * 2017-02-09 [6e2797f](https://github.com/silverstripe/silverstripe-framework/commit/6e2797ffc0e9632b60acc5a66e52aeb44f0e2b78) es for using dblib PDO driver. (Andrew O'Neil) + * 2017-02-08 [c25c443](https://github.com/silverstripe/silverstripe-framework/commit/c25c443d95fc305fb3545b1393b7da85923dcf8b) Fix minor mysql 5.7 warning in SQLQueryTest (#6608) (Damian Mooyman) + * 2017-01-18 [72b6fb4](https://github.com/silverstripe/silverstripe-framework/commit/72b6fb49b698bc3a51c8f6b32d2bf08213729493) bug: In addOrderBy method, _SortColumn will only keep the last one if there are more than 1 multi-word columns (Shawn) From 2f7f761a9c94c3aed5a2616a2530b233d942e9c3 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Tue, 30 May 2017 22:03:17 +0000 Subject: [PATCH 6/6] Added 3.5.4 changelog --- docs/en/04_Changelogs/3.5.4.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/en/04_Changelogs/3.5.4.md diff --git a/docs/en/04_Changelogs/3.5.4.md b/docs/en/04_Changelogs/3.5.4.md new file mode 100644 index 000000000..d20a30ee9 --- /dev/null +++ b/docs/en/04_Changelogs/3.5.4.md @@ -0,0 +1,28 @@ +# 3.5.4 + + + +## Change Log + +### Security + + * 2017-05-24 [41270fc](https://github.com/silverstripe/silverstripe-cms/commit/41270fcf9980c4be2529d2750c717675548eb617) Only allow HTTP(S) links for external redirector pages (Daniel Hensby) - See [ss-2017-003](http://www.silverstripe.org/download/security-releases/ss-2017-003) + * 2017-05-09 [447ce0f](https://github.com/silverstripe/silverstripe-framework/commit/447ce0f84f880c2bc969a89e4be528c53caeabe0) Lock out users who dont exist in the DB (Daniel Hensby) - See [ss-2017-002](http://www.silverstripe.org/download/security-releases/ss-2017-002) + * 2017-05-09 [61cf72c](https://github.com/silverstripe/silverstripe-cms/commit/61cf72c08dafddef416d73f943ccd45e70c5d43d) Unescaped fields in CMSPageHistroyController::compare() (Daniel Hensby) - See [ss-2017-004](http://www.silverstripe.org/download/security-releases/ss-2017-004) + +### Bugfixes + + * 2017-05-28 [16a74bc](https://github.com/silverstripe/silverstripe-framework/commit/16a74bc8a9fdee7cfb4f6f24493c271f90a76341) DataDifferencer needs to expliclty cast HTMLText values (Daniel Hensby) + * 2017-05-08 [1454072](https://github.com/silverstripe/silverstripe-cms/commit/14540729caa30dd2e782e4fd52afe518dc156ed8) Use framework 3.5 to test cms 3.5 (Sam Minnee) + * 2017-05-03 [2d138b0](https://github.com/silverstripe/silverstripe-framework/commit/2d138b0ef06bd93958cc0678a0afa95560648fb9) class name reference consistency (Gregory Smirnov) + * 2017-05-02 [2187c16](https://github.com/silverstripe/silverstripe-framework/commit/2187c160b936620621fe746a1ffe36af568b21ff) ing pagination api doc typo (3Dgoo) + * 2017-04-28 [a511e35](https://github.com/silverstripe/silverstripe-framework/commit/a511e3511cace405dab7589a3406a0858cb6edf2) #6855: Mangled JS in Requirements, escaping replacement values prior to passing to preg_replace(). (Patrick Nelson) + * 2017-04-24 [1d36f35](https://github.com/silverstripe/silverstripe-framework/commit/1d36f354e8349616c7b39fcade859fbcf0f9c362) Create Image_Cached with Injector. (Gregory Smirnov) + * 2017-04-07 [55eb7eb](https://github.com/silverstripe/silverstripe-framework/commit/55eb7ebdcc9ba767f978dff510614bbd2e0c309d) Do not insert requirements more than once in includeInHTML (Robbie Averill) + * 2017-04-05 [a7920b1](https://github.com/silverstripe/silverstripe-framework/commit/a7920b1f9866b6eb5f4bad9de84eef84b88673ad) regression from #6668 - ModelAdmin form widths (Loz Calver) + * 2017-04-05 [197bc53](https://github.com/silverstripe/silverstripe-framework/commit/197bc53c4963898d2c10621ca6d6031fdb14fe85) Add transparency percent argument to Image::generatePad to ensure transparency works from ::Pad (Robbie Averill) + * 2017-02-15 [3072591](https://github.com/silverstripe/silverstripe-framework/commit/30725916dbb0ffc66b77f26c069a86581636ae55) Array to string conversion message after CSV export (#6622) (Juan van den Anker) + * 2017-02-14 [7122e1f](https://github.com/silverstripe/silverstripe-framework/commit/7122e1fde79bdb9aad3c8714a6ce02b7ecedd735) Comments ignored by classmanifest (#6619) (Daniel Hensby) + * 2017-02-09 [6e2797f](https://github.com/silverstripe/silverstripe-framework/commit/6e2797ffc0e9632b60acc5a66e52aeb44f0e2b78) es for using dblib PDO driver. (Andrew O'Neil) + * 2017-02-08 [c25c443](https://github.com/silverstripe/silverstripe-framework/commit/c25c443d95fc305fb3545b1393b7da85923dcf8b) Fix minor mysql 5.7 warning in SQLQueryTest (#6608) (Damian Mooyman) + * 2017-01-18 [72b6fb4](https://github.com/silverstripe/silverstripe-framework/commit/72b6fb49b698bc3a51c8f6b32d2bf08213729493) bug: In addOrderBy method, _SortColumn will only keep the last one if there are more than 1 multi-word columns (Shawn)