Dynamic location for redirect... is this secure enough?

  • Bogey
  • Genius
  • Genius
  • Bogey
  • Posts: 8415
  • Loc: USA

Post 3+ Months Ago

I'm making a feature on my login page that if their is a variable named page that they get redirected to that page after logging in.

PHP Code: [ Select ]
<?php
// Default redirection page
$redirect = 'index.php';
       
// Checking if they came from another page
if(isset($_GET['page']) && !strpos('.', $_GET['page']) && !strpos('/', $_GET['page']) && file_exists($_GET['page']))
{
    $redirect = $_GET['page'];
}
 
// Sending the user to the main page
header("LOCATION: {$redirect}");
?>
  1. <?php
  2. // Default redirection page
  3. $redirect = 'index.php';
  4.        
  5. // Checking if they came from another page
  6. if(isset($_GET['page']) && !strpos('.', $_GET['page']) && !strpos('/', $_GET['page']) && file_exists($_GET['page']))
  7. {
  8.     $redirect = $_GET['page'];
  9. }
  10.  
  11. // Sending the user to the main page
  12. header("LOCATION: {$redirect}");
  13. ?>

So far I got that... is it secure enough?

Reason I'm doing this is that if they user clicks login they get redirected to the page they came from after logging in so they won't loose their position. I think that will work pretty well.
  • SpooF
  • ٩๏̯͡๏۶
  • Bronze Member
  • User avatar
  • Posts: 3422
  • Loc: Richland, WA

Post 3+ Months Ago

I don't see why that wouldn't be secure. Its just a redirect, you arn't including it into your page so I don't see how someone could exploit it.


I don't believe this will work if you have any sort of query string on the file though.
  • Bogey
  • Genius
  • Genius
  • Bogey
  • Posts: 8415
  • Loc: USA

Post 3+ Months Ago

SpooF wrote:
I don't see why that wouldn't be secure. Its just a redirect, you arn't including it into your page so I don't see how someone could exploit it.

Alright, thanks
SpooF wrote:
I don't believe this will work if you have any sort of query string on the file though.

What do you mean? Whatever you mean is right lol... I can't seem to get it to redirect to the place $_GET['page'] tells it to
  • joebert
  • Fart Bubbles
  • Genius
  • User avatar
  • Posts: 13504
  • Loc: Florida

Post 3+ Months Ago

You could probably do without checking for the dot, and using "!empty()" instead of "isset()" will save the system from needing to proceed in an instance where page is set, but doesn't contain a value, but it seems secure.

The only way I can think of that this might be bypassable, is if there's some sort of multi-byte characters that your operating system would interpret as and automatically convert to directory separators, dots, or something along those lines. If that were possible, I suppose a phishing page could be setup that redirects the user to an external website using your redirect.

Basically someone would setup a link to your login page and include this malformed URL pointing to their own website. A user may simply glance at the first domain name they see in this login link, which would be your domain and no cause for concern. Meanwhile, this malformed link would redirect the user to a different website which looks exactly like your websites login page but with an error message along the lines of "incorrect password, please make sure you don't have CAPSLOCK on and try again" which since they just came from your login page, might ease anyone's suspicions and make them enter their username and password into this phishing site. After collecting your password the phishing site could redirect the visitor back to your site and to the page they thought they were going to in the first place and never realize what just happened until it was too late.

Actually, as I was typing this, urlencoded directory separators came to mind. However, since you've got a file_exists() call in there, this whole idea is pretty much invalid. As long as critical files such as maybe a "config.php" are protected from being viewed by browsers to begin with, it doesn't matter if you can be redirected to them. Besides, it would be easier to just type the address in the browsers address bar in that case.
  • Bogey
  • Genius
  • Genius
  • Bogey
  • Posts: 8415
  • Loc: USA

Post 3+ Months Ago

Alright, I have the following (Thanks for the headsup on the "isset()" and the dot... I took this code from another place on my site which requires them).

Anyway... what I have is the following:

PHP Code: [ Select ]
<?php
// Default redirection page
$redirect = 'index.php';
       
// Checking if they came from another page
if(!empty($_GET['page']) && !strpos('/', $_GET['page']) && file_exists($_GET['page']))
{
    $redirect = $_GET['page'];
}
 
// Sending the user to the main page
header("LOCATION: {$redirect}");
?>
  1. <?php
  2. // Default redirection page
  3. $redirect = 'index.php';
  4.        
  5. // Checking if they came from another page
  6. if(!empty($_GET['page']) && !strpos('/', $_GET['page']) && file_exists($_GET['page']))
  7. {
  8.     $redirect = $_GET['page'];
  9. }
  10.  
  11. // Sending the user to the main page
  12. header("LOCATION: {$redirect}");
  13. ?>

And that doesn't redirect the user to any page specified by "$_GET['page']" like it should... just goes to "index.php".

The header redirect is placed before any of the HTML is parsed.

[EDIT] I've also tried the following...

PHP Code: [ Select ]
<?php
        // Checking if they came from another page
        if(!empty($_GET['page']))
        {
            $redirect = $_GET['page'];
        }
        else
        {
            $redirect = 'index.php';
        }
        echo $redirect;
        // Sending the user to the main page
        //header("LOCATION: {$redirect}");
?>
  1. <?php
  2.         // Checking if they came from another page
  3.         if(!empty($_GET['page']))
  4.         {
  5.             $redirect = $_GET['page'];
  6.         }
  7.         else
  8.         {
  9.             $redirect = 'index.php';
  10.         }
  11.         echo $redirect;
  12.         // Sending the user to the main page
  13.         //header("LOCATION: {$redirect}");
  14. ?>

And that doesn't work as well... it's always index.php no matter what
  • joebert
  • Fart Bubbles
  • Genius
  • User avatar
  • Posts: 13504
  • Loc: Florida

Post 3+ Months Ago

Have you been manually adding ?page=*** to your URL when testing, or have you been using the login form?

If you've been using the login form, make sure it's including your page variable.
  • Bogey
  • Genius
  • Genius
  • Bogey
  • Posts: 8415
  • Loc: USA

Post 3+ Months Ago

joebert wrote:
Have you been manually adding ?page=*** to your URL when testing, or have you been using the login form?

If you've been using the login form, make sure it's including your page variable.

I've being manually adding ?page=*** to the URL, press enter, then login and get redirected to index.php no matter what I put as the value to page... I have no idea what's going on...

[EDIT] I tried the following...

PHP Code: [ Select ]
<?php
$redirect = $_GET['page'];
 
// Sending the user to the main page
header("LOCATION: {$redirect}");
?>
  1. <?php
  2. $redirect = $_GET['page'];
  3.  
  4. // Sending the user to the main page
  5. header("LOCATION: {$redirect}");
  6. ?>


And I get the following error

Quote:
Notice: Undefined index: page in /var/www/Projects/CMS/login.php on line 96


The URL looks like...

Quote:
http://localhost/Projects/CMS/login.php?page=test.php


Not sure what to do here...

[EDIT #2] Not sure what to do or what to look for... here is the entire file... maybe you could notice something that I can't...

PHP Code: [ Select ]
<?php
 
/*
 * login.php
 *
 *
 */
 
// Security first
define('IN_CMS', true);
 
// Including the global file... without it, we don't have the system.
require_once('includes/init.php');
 
// Checking if the user is already logged in
if($sess->logged() != false)
{
    header("LOCATION: index.php");
}
 
// Defining the variables for the header
$tpl->title = "Log In to {$tpl->header}";
$tpl->description = "Log In to {$tpl->header} to be able to see and use all the member's only items.";
 
// Setting the error session (if required)
if(!isset($_SESSION['login_trials']))
{
    $_SESSION['login_trials'] = 0;
}
 
// Checking if the user has reached the error limit of failed login attempts
if($_SESSION['login_trials'] >= 4)
{
    $tpl->securityCode = true;
}
 
// Doing the page processing
if(isset($_POST['Submit']))
{
    // Initiating the error array
    $error = array();
 
    // Checking the username
    if($_POST['username'] == '' OR strlen($_POST['username']) > 20)
    {
        $error[] = "You need to put in a valid username that is under 20 characters.";
    }
    else
    {   // checking if the username exists
        $sql = $db->build_key_query(array('SELECT'  => 'username',
                                          'FROM'    => USER_TABLE,
                                          'WHERE'   => array("username" => $_POST['username'],
                                                             "password" => md5($_POST['password']))));
       
        // Counting if we have a username by the submitted username
        if($db->num_rows($sql) == 0)
        {
            $error[] = "The username / password combination was incorrect. Please try again.";
           
            // Making sure that the user doesn't attempt more then the defined times
            $_SESSION['login_trials'] += 1;
        }
    }
   
    // Checking the Password
    if($_POST['password'] == '' OR strlen($_POST['password']) > 20)
    {
        $error[] = "You need to put in a valid password that is under 20 characters.";
    }
   
    // Checking if the user has reached the error limit of failed login attempts
    if($_SESSION['login_trials'] > 5)
    {
        // Checking the CAPTCHA / Security Code
        if(strtolower($_SESSION['qwerty']) != strtolower($_POST['qwerty']))
        {
            $error[] = "You need to type in the Correct Security code. It is not case sensitive.";
        }
    }
   
    // Checking for any errors in the submittion
    if(count($error) > 0)
    {
        $tpl->formErrors = $error;
    }
    else
    {
        // Removing the failed login trials session entry
        unset($_SESSION['login_trials']);
       
        // We are logging the user in here since everything checked out
        $_SESSION['u_logged_in'] = $db->_get_('userID', USER_TABLE, array("username" => $_POST['username'],
                                                                          "password" => md5($_POST['password'])));
       
        $redirect = $_GET['page'];
 
        // Sending the user to the main page
        header("LOCATION: {$redirect}");
    }
}
 
// Setting the required template variables to display
$tpl->loginFormMethod = 'post';
$tpl->loginFormAction = 'login.php';
 
// Displaying the page here
$tpl->display('header', 'login', 'footer');
?>
  1. <?php
  2.  
  3. /*
  4.  * login.php
  5.  *
  6.  *
  7.  */
  8.  
  9. // Security first
  10. define('IN_CMS', true);
  11.  
  12. // Including the global file... without it, we don't have the system.
  13. require_once('includes/init.php');
  14.  
  15. // Checking if the user is already logged in
  16. if($sess->logged() != false)
  17. {
  18.     header("LOCATION: index.php");
  19. }
  20.  
  21. // Defining the variables for the header
  22. $tpl->title = "Log In to {$tpl->header}";
  23. $tpl->description = "Log In to {$tpl->header} to be able to see and use all the member's only items.";
  24.  
  25. // Setting the error session (if required)
  26. if(!isset($_SESSION['login_trials']))
  27. {
  28.     $_SESSION['login_trials'] = 0;
  29. }
  30.  
  31. // Checking if the user has reached the error limit of failed login attempts
  32. if($_SESSION['login_trials'] >= 4)
  33. {
  34.     $tpl->securityCode = true;
  35. }
  36.  
  37. // Doing the page processing
  38. if(isset($_POST['Submit']))
  39. {
  40.     // Initiating the error array
  41.     $error = array();
  42.  
  43.     // Checking the username
  44.     if($_POST['username'] == '' OR strlen($_POST['username']) > 20)
  45.     {
  46.         $error[] = "You need to put in a valid username that is under 20 characters.";
  47.     }
  48.     else
  49.     {   // checking if the username exists
  50.         $sql = $db->build_key_query(array('SELECT'  => 'username',
  51.                                           'FROM'    => USER_TABLE,
  52.                                           'WHERE'   => array("username" => $_POST['username'],
  53.                                                              "password" => md5($_POST['password']))));
  54.        
  55.         // Counting if we have a username by the submitted username
  56.         if($db->num_rows($sql) == 0)
  57.         {
  58.             $error[] = "The username / password combination was incorrect. Please try again.";
  59.            
  60.             // Making sure that the user doesn't attempt more then the defined times
  61.             $_SESSION['login_trials'] += 1;
  62.         }
  63.     }
  64.    
  65.     // Checking the Password
  66.     if($_POST['password'] == '' OR strlen($_POST['password']) > 20)
  67.     {
  68.         $error[] = "You need to put in a valid password that is under 20 characters.";
  69.     }
  70.    
  71.     // Checking if the user has reached the error limit of failed login attempts
  72.     if($_SESSION['login_trials'] > 5)
  73.     {
  74.         // Checking the CAPTCHA / Security Code
  75.         if(strtolower($_SESSION['qwerty']) != strtolower($_POST['qwerty']))
  76.         {
  77.             $error[] = "You need to type in the Correct Security code. It is not case sensitive.";
  78.         }
  79.     }
  80.    
  81.     // Checking for any errors in the submittion
  82.     if(count($error) > 0)
  83.     {
  84.         $tpl->formErrors = $error;
  85.     }
  86.     else
  87.     {
  88.         // Removing the failed login trials session entry
  89.         unset($_SESSION['login_trials']);
  90.        
  91.         // We are logging the user in here since everything checked out
  92.         $_SESSION['u_logged_in'] = $db->_get_('userID', USER_TABLE, array("username" => $_POST['username'],
  93.                                                                           "password" => md5($_POST['password'])));
  94.        
  95.         $redirect = $_GET['page'];
  96.  
  97.         // Sending the user to the main page
  98.         header("LOCATION: {$redirect}");
  99.     }
  100. }
  101.  
  102. // Setting the required template variables to display
  103. $tpl->loginFormMethod = 'post';
  104. $tpl->loginFormAction = 'login.php';
  105.  
  106. // Displaying the page here
  107. $tpl->display('header', 'login', 'footer');
  108. ?>
  • SpooF
  • ٩๏̯͡๏۶
  • Bronze Member
  • User avatar
  • Posts: 3422
  • Loc: Richland, WA

Post 3+ Months Ago

That error makes it sound like there is no index 'page' in the $_GET variable.
  • Bogey
  • Genius
  • Genius
  • Bogey
  • Posts: 8415
  • Loc: USA

Post 3+ Months Ago

Yes... I have no idea why though as I'm passing it through the URL... why would it be doing that nonsense?

I put echo $redirect; on line 96 and it didn't echo anything... but If I put echo $_GET['page']; on line 20 it echoes 'test.php'... what the?
  • Bogey
  • Genius
  • Genius
  • Bogey
  • Posts: 8415
  • Loc: USA

Post 3+ Months Ago

Nevermind, my simple brain overlooked that no matter what I put into the URL, the form action doesn't have that :(

:lol: Anyway... I got it fixed.

My other problem... $_SERVER['PHP_SELF'] returns /path/to/file.php and I'm trying to keep it secure by not allowing the dashes...

Should I allow the dashes or should I use regex to receive only the "file.php" part? If I need to use regex there's another problem... I don't know regex...
  • joebert
  • Fart Bubbles
  • Genius
  • User avatar
  • Posts: 13504
  • Loc: Florida

Post 3+ Months Ago

Don't use $_SERVER['PHP_SELF'], use $_SERVER['SCRIPT_FILENAME'] instead. SCRIPT_FILENAME will never contain PATH_INFO (ie "/index.php/path-info") and it also takes into consideration symlinks on the server, whereas the __FILE__ constant does not.

Use basename() to retrieve the "file.php" part from SCRIPT_FILENAME, and dirname() to get the working directory.
  • Bogey
  • Genius
  • Genius
  • Bogey
  • Posts: 8415
  • Loc: USA

Post 3+ Months Ago

Oh ok, thanks for the heads up. That works perfectly :)

Post Information

  • Total Posts in this topic: 12 posts
  • Users browsing this forum: No registered users and 110 guests
  • You cannot post new topics in this forum
  • You cannot reply to topics in this forum
  • You cannot edit your posts in this forum
  • You cannot delete your posts in this forum
  • You cannot post attachments in this forum
 
cron
 

© 1998-2014. Ozzu® is a registered trademark of Unmelted, LLC.