hotline#143707 : Security : Open Redirect issue fixed

VERSIONS_HOTLINE/143707

+ - ticket #143707 : Sécurité : les redirections au login vers des urls externes ne sont plus permises
\ No newline at end of file

application/modules/admin/controllers/AuthController.php

@@ -35,7 +35,7 @@ class Admin_AuthController extends Zend_Controller_Action {
 
   function loginAction() {
     $this->view->message = '';
-    $this->view->redirect = $this->_getParam('redirect');
+    $this->view->redirect = $this->_processRedirect();
 
     if (!$this->_request->isPost())
       return;
@@ -53,10 +53,23 @@ class Admin_AuthController extends Zend_Controller_Action {
     if (!$auth->authenticateLoginPassword($username, $password, [$auth->newAuthDb()]))
       return;
 
-    $this->_redirect($this->_request->getPost('redirect', 'admin/'));
+    if ($this->view->redirect)
+      $this->_redirect($this->view->redirect);
   }
 
 
+  protected function _processRedirect(){
+    if (! $redirect_candidate = $this->_getParam('redirect'))
+      return '/admin/';
+
+    if (Class_Url::isABokehUrl($redirect_candidate))
+      return $redirect_candidate;
+
+    return '/admin/';
+  }
+
+
+
   function logoutAction() {
     ZendAfi_Auth::getInstance()->clearIdentity();
     $this->_redirect('admin/');

library/Class/Url.php

@@ -97,6 +97,12 @@ class Class_Url {
   }
 
 
+  public static function isABokehUrl($string) {
+    return ((strpos('/',$string) == 0)
+      || (strpos(static::absolute([], null, true), $string) == 0));
+  }
+
+
   protected static function _isSecure() {
     return array_key_exists('HTTPS', $_SERVER)
       && $_SERVER['HTTPS']

tests/application/modules/admin/controllers/AdminAuthControllerTest.php

@@ -126,9 +126,9 @@ class AdminAuthControllerSuccessfulLoggedTest extends AbstractControllerTestCase
     $this->postDispatch('/admin/auth/login',
                         ['username' => 'foo',
                          'password' => 'bar',
-                         'redirect' => 'http://www.fsf.org']);
+                         'redirect' => '/admin/newsletter']);
 
-    $this->assertRedirectTo('http://www.fsf.org');
+    $this->assertRedirectTo('/admin/newsletter');
   }
 }
 

tests/scenarios/Security/AdminAuthTest.php

@@ -36,3 +36,68 @@ class Security_AdminAuthTest extends Admin_AbstractControllerTestCase {
     $this->assertNotXPathContentContains('//script', '_injected');
   }
 }
+
+
+class AdminAuthControllerSuccessfulLoggedExternalRedirectTest extends AbstractControllerTestCase {
+
+  protected
+    $_storm_default_to_volatile = true,
+    $_auth,
+    $_auth_db_adapter;
+
+
+  public function setUp() {
+    parent::setUp();
+    Class_Users::newInstanceWithId(2,
+                                   ['nom' => 'Marcel','login' =>'foo', 'password' => 'bar'])
+      ->beAdminPortail()
+      ->assertSave();
+
+    $this->_auth = $this->mock()
+                        ->whenCalled('authenticateLoginPassword')
+                        ->with('foo', 'bar', [$this->_auth_db_adapter])
+                        ->answers(true)
+                        ->whenCalled('getIdentity')
+                        ->answers(Class_Users::find(2))
+                        ->whenCalled('hasIdentity')
+                        ->answers(true)
+                        ->whenCalled('newAuthDb')->answers($this->_auth_db_adapter);
+
+    ZendAfi_Auth::setInstance($this->_auth);
+  }
+
+
+  public function tearDown() {
+    ZendAfi_Auth::setInstance(null);
+    parent::tearDown();
+  }
+
+
+  /** @test */
+  public function withExternalUrlShouldRedirectToAdmin() {
+    $this->postDispatch('/admin/auth/login',
+                        ['username' => 'foo',
+                         'password' => 'bar',
+                        'redirect' => 'https://fsf.org']);
+    xdebug_break();
+    $this->assertRedirectTo('/admin/');
+  }
+
+
+  /** @test */
+  public function withInternalUrlShouldRedirectToIt() {
+    $this->postDispatch('/admin/auth/login',
+                        ['username' => 'foo',
+                         'password' => 'bar',
+                         'redirect' => Class_Url::absolute(['module'=>'admin',
+                                                            'controller'=>'users'],
+                                                           null,
+                                                           true)
+                        ]);
+
+    $this->assertRedirectTo(Class_Url::absolute(['module'=>'admin',
+                                                 'controller'=>'users'],
+                                                null,
+                                                true));
+  }
+}