tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

BUGFIX Fixed redirection to external URLs through Security/login with BackURL parameter

Browse Source 
git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@71708 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
Ingo Schommer 2009-02-11 21:08:28 +00:00
parent 74ab33f23b
commit 6d708765fe
2 changed files with 47 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
security/MemberLoginForm.php
View File

@ -111,7 +111,6 @@ class MemberLoginForm extends LoginForm {
if($this->performLogin($data)) { if($this->performLogin($data)) {
Session::clear('SessionForms.MemberLoginForm.Email'); Session::clear('SessionForms.MemberLoginForm.Email');
Session::clear('SessionForms.MemberLoginForm.Remember'); Session::clear('SessionForms.MemberLoginForm.Remember');
if(Member::currentUser()->isPasswordExpired()) { if(Member::currentUser()->isPasswordExpired()) {
if(isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['BackURL']) { if(isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['BackURL']) {
Session::set('BackURL', $backURL); Session::set('BackURL', $backURL);
@ -121,9 +120,17 @@ class MemberLoginForm extends LoginForm {
$cp->sessionMessage('Your password has expired. Please choose a new one.', 'good'); $cp->sessionMessage('Your password has expired. Please choose a new one.', 'good');
Director::redirect('Security/changepassword'); Director::redirect('Security/changepassword');
} elseif(isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['BackURL']) { } elseif(
Session::clear("BackURL"); isset($_REQUEST['BackURL'])
Director::redirect($backURL); && $_REQUEST['BackURL']
&& (
// absolute redirection URLs may cause spoofing
!Director::is_absolute_url($_REQUEST['BackURL'])
// absolute URLs on the current domain are allowed
|| strpos($_REQUEST['BackURL'], Director::absoluteBaseURL()) !== FALSE
)
) {
Director::redirect($_REQUEST['BackURL']);
} else { } else {
$member = Member::currentUser(); $member = Member::currentUser();
if($member) { if($member) {

38
tests/security/SecurityTest.php
View File

@ -37,6 +37,40 @@ class SecurityTest extends FunctionalTest {
parent::tearDown(); parent::tearDown();
} }
function testExternalBackUrlRedirectionDisallowed() {
$page = new SiteTree();
$page->URLSegment = 'testpage';
$page->Title = 'Testpage';
$page->write();
$page->publish('Stage','Live');
// Test internal relative redirect
$response = $this->doTestLoginForm('noexpiry@silverstripe.com', '1nitialPassword', 'testpage');
$this->assertEquals(302, $response->getStatusCode());
$this->assertRegExp('/testpage/', $response->getHeader('Location'),
"Internal relative BackURLs work when passed through to login form"
);
// Log the user out
$this->session()->inst_set('loggedInAs', null);
// Test internal absolute redirect
$response = $this->doTestLoginForm('noexpiry@silverstripe.com', '1nitialPassword', Director::absoluteBaseURL() . 'testpage');
// for some reason the redirect happens to a relative URL
$this->assertRegExp('/^' . preg_quote(Director::absoluteBaseURL(), '/') . 'testpage/', $response->getHeader('Location'),
"Internal absolute BackURLs work when passed through to login form"
);
// Log the user out
$this->session()->inst_set('loggedInAs', null);
// Test external redirect
$response = $this->doTestLoginForm('noexpiry@silverstripe.com', '1nitialPassword', 'http://myspoofedhost.com');
$this->assertNotRegExp('/^' . preg_quote('http://myspoofedhost.com', '/') . '/', $response->getHeader('Location'),
"Redirection to external links in login form BackURL gets prevented as a measure against spoofing attacks"
);
// Log the user out
$this->session()->inst_set('loggedInAs', null);
}
/** /**
* Test that the login form redirects to the change password form after logging in with an expired password * Test that the login form redirects to the change password form after logging in with an expired password
*/ */
@ -183,8 +217,8 @@ class SecurityTest extends FunctionalTest {
* Execute a log-in form using Director::test(). * Execute a log-in form using Director::test().
* Helper method for the tests above * Helper method for the tests above
*/ */
function doTestLoginForm($email, $password) { function doTestLoginForm($email, $password, $backURL = 'test/link') {
$this->session()->inst_set('BackURL', 'test/link'); $this->session()->inst_set('BackURL', $backURL);
$this->get('Security/login'); $this->get('Security/login');
return $this->submitForm( return $this->submitForm(