How secure is my code?

Feel free to comment/suggest.....

[PumpkinHead]

Hi all,

After my last post, I had my eyes opened to many issues in my code. I have now redesigned my code so feel free to comment on any security vulnerbilites or suggest better ways of doing things.

The following code is a simple login script...

index.php
---------

<?php
session_start();
if (isset($_SESSION['logged_in']))
{
if (($_SESSION['logged_in']) == 'Y')
{
redirect();
}
else
{
display_login();
}
}
else
{
if (isset($_POST['submit']))
{
check_database();
}
else
{
display_login();
}
}
// *****************************************************************
function display_login() 
{
echo "Please enter your username and password...";
echo "<form action='" . $_SERVER['PHP_SELF'] . "' method='post' enctype='multipart/form-data'>";
echo "<input name='var_userid' type='text'><br>";
echo "<input name='var_pass' value='' type='password'><br>";
echo "<input name='submit' value='Login' type='submit'>";
echo "</form>";
}
// *****************************************************************
function quote_smart($value)
{
  // Stripslashes
  if (get_magic_quotes_gpc()) {
    $value = stripslashes($value);
  }
  // Quote if not integer
  if (!is_numeric($value)) {
    $value = "'" . mysql_real_escape_string($value) . "'";
  }
  return $value;
}
// *****************************************************************
function check_database() 
{
require "db_connect.php";
$query = sprintf("SELECT * FROM users WHERE username='%s' AND password='%s'",
  mysql_real_escape_string($_POST['var_userid']),
  mysql_real_escape_string($_POST['var_pass']));
 
$result = mysql_query($query) or die (mysql_error()); 
if (mysql_num_rows($result) > 0)
{
$_SESSION['logged_in'] = 'Y';
redirect();
}
else
{
$_SESSION['logged_in'] = 'N';
display_login();
echo "INVALID USERNAME/PASSWORD";
}
}
// *****************************************************************
function redirect()
{
if (isset($_REQUEST['page']))
{
if (($_REQUEST['page']) == "") {display_menu();}
if (($_REQUEST['page']) == "secret_link"){require("secret_page.php");}
}
else
{
display_menu();
}
}
// *****************************************************************
function display_menu()
{
echo "You are now logged in." . "<br>";
echo "<a href='?page=secret_link'>Secret Link</a>" . "<br>";
}
?>

db_connect.php
--------------

<?php
$db_conn = mysql_connect("localhost", "username", "password") or die("unable to connect to the database");
mysql_select_db("my_db", $db_conn) or die("unable to select the database");
?>

secret_page.php
---------------

<?php
echo "<h1>This is the secret page</h1>";
?>

[R e b r a n d t]

So.. if i for example enter [yourdomain...] i will pass all your login/session checking. Don't forget to include session checking in EVERY script.

[PumpkinHead]

How would someone know the name of the 'secret_page.php'?

[R e b r a n d t]

Sometimes you have a link to a secret_page.php from your index.php.
Sometimes you have an error in your index.php script that can reveal the secret_page.php.
Sometimes your php engine is down and visitors see the source code of index.php and can find out the secret_page.php
Sometimes you have some outbound links in your secret_page.php. When you click them then some other site web server logs save a reference to your secret_page.php.
Sometimes your secret_page.php is named with easy to guess words: secret_page, secret, admin, adm, test, setup, config, etc. That way some evil crawlers/scanners can accidently find your secret_page.php
Sometimes evil people can guess you have the secret_page.php
Sometimes...

[PumpkinHead]

Ok, well I don't ever see any of those things happening, but can anyone suggest a way around this or do I need to start again from scratch taking into consideration the things said above?

[PumpkinHead]

Would the following cure this problem?

secret_page.php
---------------

<?php
if (isset($_SESSION['logged_in']))
{
if (($_SESSION['logged_in'])!= 'Y')
{
echo "You are not authrorised to view this page.";
exit();
}
}
else
{
echo "You are not authrorised to view this page.";
exit();
}
echo "<h1>This is the secret page</h1>";
?>

[jd01]

Try this:

Change This:
function display_menu()
{
echo "You are now logged in." . "<br>";
echo "<a href='?page=secret_link'>Secret Link</a>" . "<br>";
}
?>

To This:

include "you_cant_open_me.php";

Make you_cant_open_me.php this:
<?
function display_menu()
{
echo "You are now logged in." . "<br>";
echo "<a href='?page=secret_link'>Secret Link</a>" . "<br>";
}
?>

And put this in your .htaccess:
RewriteEngine ON
RewriteRule ^yourdirectory/you_cant_see_me.php - [F]

Then try to open you_cant_open_me.php...

But the main page will still run.

Justin

Edit: Brain Clutter - Moved entire function