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

BUGFIX BUGFIX Fixed redirection to external URLs through Security/login with BackURL parameter (merged from trunk

Browse Source 
git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.3@71709 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
Ingo Schommer 2009-02-11 21:12:20 +00:00 committed by Sam Minnee
parent c799947171
commit 931182d0b2
4 changed files with 72 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
core/control/Director.php
View File

@ -461,6 +461,20 @@ class Director {
return preg_match('/^[a-zA-Z]:[\\\\\/]/', $path) == 1;
}
/**
* Checks if a given URL is absolute (e.g. starts with 'http://' etc.).
*
* @param string $url
* @return boolean
*/
public static function is_absolute_url($url) {
$url = trim($url);
// remove all query strings to avoid parse_url choking on URLs like 'test.com?url=http://test.com'
$url = preg_replace('/.*(\?.*)/', '', $url);
$parsed = parse_url($url);
return (isset($parsed['scheme']));
}
/**
* Given a filesystem reference relative to the site root, return the full file-system path.
*

15
security/MemberLoginForm.php
View File

@ -111,7 +111,6 @@ class MemberLoginForm extends LoginForm {
if($this->performLogin($data)) {
Session::clear('SessionForms.MemberLoginForm.Email');
Session::clear('SessionForms.MemberLoginForm.Remember');
if(Member::currentUser()->isPasswordExpired()) {
if(isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['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');
Director::redirect('Security/changepassword');
} elseif(isset($_REQUEST['BackURL']) && $backURL = $_REQUEST['BackURL']) {
Session::clear("BackURL");
Director::redirect($backURL);
} elseif(
isset($_REQUEST['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 {
$member = Member::currentUser();
if($member) {

11
tests/control/DirectorTest.php
View File

@ -68,5 +68,16 @@ class DirectorTest extends SapphireTest {
}
}
public function testIsAbsoluteUrl() {
$this->assertTrue(Director::is_absolute_url('http://test.com'));
$this->assertTrue(Director::is_absolute_url('https://test.com'));
$this->assertTrue(Director::is_absolute_url(' https://test.com/testpage '));
$this->assertFalse(Director::is_absolute_url('test.com/testpage'));
$this->assertTrue(Director::is_absolute_url('ftp://test.com'));
$this->assertFalse(Director::is_absolute_url('/relative'));
$this->assertFalse(Director::is_absolute_url('relative'));
$this->assertFalse(Director::is_absolute_url('/relative/?url=http://test.com'));
}
}
?>

38
tests/security/SecurityTest.php
View File

@ -37,6 +37,40 @@ class SecurityTest extends FunctionalTest {
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
*/
@ -183,8 +217,8 @@ class SecurityTest extends FunctionalTest {
* Execute a log-in form using Director::test().
* Helper method for the tests above
*/
function doTestLoginForm($email, $password) {
$this->session()->inst_set('BackURL', 'test/link');
function doTestLoginForm($email, $password, $backURL = 'test/link') {
$this->session()->inst_set('BackURL', $backURL);
$this->get('Security/login');
return $this->submitForm(