Permalink
Browse files

MINOR Redirect user to homepage if the BackURL have been set to anoth…

…er site.

This might indicatate a spoofing attack. I also extracted code into it's own method to make it easier to read.
  • Loading branch information...
1 parent 6db8306 commit 7a4c7a6e23dee40212db41012f761f4fd8fad6cd @stojg stojg committed Oct 27, 2011
Showing with 71 additions and 41 deletions.
  1. +71 −41 security/MemberLoginForm.php
@@ -123,48 +123,12 @@ protected function getMessageFromSession() {
*/
public function dologin($data) {
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);
- }
-
- $cp = new ChangePasswordForm($this->controller, 'ChangePasswordForm');
- $cp->sessionMessage('Your password has expired. Please choose a new one.', 'good');
-
- Director::redirect('Security/changepassword');
- } elseif(
- isset($_REQUEST['BackURL'])
- && $_REQUEST['BackURL']
- // absolute redirection URLs may cause spoofing
- && Director::is_site_url($_REQUEST['BackURL'])
- ) {
- Director::redirect($_REQUEST['BackURL']);
- } elseif (Security::default_login_dest()) {
- Director::redirect(Director::absoluteBaseURL() . Security::default_login_dest());
- } else {
- $member = Member::currentUser();
- if($member) {
- $firstname = Convert::raw2xml($member->FirstName);
-
- if(!empty($data['Remember'])) {
- Session::set('SessionForms.MemberLoginForm.Remember', '1');
- $member->logIn(true);
- } else {
- $member->logIn();
- }
-
- Session::set('Security.Message.message',
- sprintf(_t('Member.WELCOMEBACK', "Welcome Back, %s"), $firstname)
- );
- Session::set("Security.Message.type", "good");
- }
- Director::redirectBack();
- }
+ $this->logInUserAndRedirect($data);
} else {
- Session::set('SessionForms.MemberLoginForm.Email', $data['Email']);
- Session::set('SessionForms.MemberLoginForm.Remember', isset($data['Remember']));
+ if(array_key_exists('Email', $data)){
+ Session::set('SessionForms.MemberLoginForm.Email', $data['Email']);
+ Session::set('SessionForms.MemberLoginForm.Remember', isset($data['Remember']));
+ }
if(isset($_REQUEST['BackURL'])) $backURL = $_REQUEST['BackURL'];
else $backURL = null;
@@ -182,6 +146,72 @@ public function dologin($data) {
}
}
+ /**
+ * Login in the user and figure out where to redirect the browser.
+ *
+ * The $data has this format
+ * array(
+ * 'AuthenticationMethod' => 'MemberAuthenticator',
+ * 'Email' => 'sam@silverstripe.com',
+ * 'Password' => '1nitialPassword',
+ * 'BackURL' => 'test/link',
+ * [Optional: 'Remember' => 1 ]
+ * )
+ *
+ * @param array $data
+ * @return void
+ */
+ protected function logInUserAndRedirect($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);
+ }
+ $cp = new ChangePasswordForm($this->controller, 'ChangePasswordForm');
+ $cp->sessionMessage('Your password has expired. Please choose a new one.', 'good');
+ Director::redirect('Security/changepassword');
+ return;
+ }
+
+ // Absolute redirection URLs may cause spoofing
+ if(isset($_REQUEST['BackURL']) && $_REQUEST['BackURL'] && Director::is_site_url($_REQUEST['BackURL']) ) {
+ Director::redirect($_REQUEST['BackURL']);
+ return;
+ }
+
+ // Spoofing attack, redirect to homepage instead of spoofing url
+ if(isset($_REQUEST['BackURL']) && $_REQUEST['BackURL'] && !Director::is_site_url($_REQUEST['BackURL'])) {
+ Director::redirect(Director::absoluteBaseURL());
+ return;
+ }
+
+ // If a default login dest has been set, redirect to that.
+ if (Security::default_login_dest()) {
+ Director::redirect(Director::absoluteBaseURL() . Security::default_login_dest());
+ return;
+ }
+
+ // Redirect the user to the page where he came from
+ $member = Member::currentUser();
+ if($member) {
+ $firstname = Convert::raw2xml($member->FirstName);
+ if(!empty($data['Remember'])) {
+ Session::set('SessionForms.MemberLoginForm.Remember', '1');
+ $member->logIn(true);
+ } else {
+ $member->logIn();
+ }
+
+ Session::set('Security.Message.message',
+ sprintf(_t('Member.WELCOMEBACK', "Welcome Back, %s"), $firstname)
+ );
+ Session::set("Security.Message.type", "good");
+ }
+ Controller::curr()->redirectBack();
+ }
+
/**
* Log out form handler method

0 comments on commit 7a4c7a6

Please sign in to comment.