Login help

[Axsuul]

Hi,

First of all, here is my code:

// document roots

$adminroot = ($_SERVER["DOCUMENT_ROOT"] . "/admin/");        // admin root

// pages

$page = $_GET["page"];        // admin pages
    if ( $page == "" ) 
        { 
                   include($adminroot . "admin_login.php"); 
} 
    else if ( $page == "main" ) 
        { 
                  include($adminroot . "admin_main.php"); 
} 
    else if ( $page == "news" ) 
        { 
                  echo "About page goes here."; 
} 

$valid = array('admin.php', 'admin_main.php', 'admin_login.php');        // states what are valid pages for security reasons

if ( in_array($page, $valid) )        // checks if page is valid from an array
    {                                
        include($page);
{
    else ( !in_array($page, $valid) )
        {
               die("Page non-existant.");
}

First of all, what I'm trying to do is before I get a page I want to make sure and validate that it's a valid page to retrieve to prevent intruders from simply typing "admin.php?page=/etc/pass". Well as you might have concluded, the above code doesn't appear to work. Can anyone help me on my way on what I'm trying to do? Thanks in advance.

[steadyguy]

I think your problem is in the line:

$page = $_GET["page"];

Because later on, you include the submitted filename.

Instead of letting the user type in a page, and then including whatever they typed in, it would be more secure and easier to to give the page a handle that is different from the filename. You could then compare the handle to a list of handles in an array, and then give them the appropriate page. If the array is associative with the keys as the handles, and the values as the filenames, you could do something like:

$page = $_GET['page'];
$valid = array('main'=>'main.php', 'news'=>'news.php'); // and so on
$flag = 0;
foreach ($news as $key=>$value) {
    if ($key == $page) {
        include_once($value);
        $flag = 1;
        break;
    }
}
if ($flag != 1)
    print 'Page not available.';

[Axsuul]

Hey, thanks for the feedback. Your suggestion is good but I was also wondering when I looked at your code what the $news string in:

foreach ($news as $key=>$value) { 

was for. And also, what if I wanted whenever a user could type "domain/admin.php" it would also point to a page to login without having to type "domain/admin.php?page=login" is that possible as well? Thanks for your time

[steadyguy]

Oops ... sorry, that should be $valid ... I think I renamed the array while I was writing the post. My bad.

As for the pointing to the login page by default, just check for a $GET['page'] value before you run that foreach:

$page = $_GET['page'];
if (!$page) 
    include_once('login.php');
else {
    $valid = array('main'=>'main.php', 'news'=>'news.php'); // and so on
    $flag = 0;
    foreach ($valid as $key=>$value) {
        if ($key == $page) {
            include_once($value);
            $flag = 1;
            break;
        }
    }
    if ($flag != 1)
        print 'Page not available.';
}

Does that make a bit more sense?