Topic: Secure Login Area

[wmguk]

Hi All,

I have a secure login system installed, however is isnt secure as someone has managed to access one of the files which removes images in the system.

This is the login code:

<?
ob_start();
session_start();
include('../includes/dbconn.php');  

$user=$_REQUEST['user'];
$pass=$_REQUEST['pass'];

$sql="select * from admin where username='$user' and password='$pass'";
$result = mysql_query($sql) or die("Query failed : " . mysql_error());
$row=mysql_fetch_assoc($result);

$no=mysql_num_rows($result);

if($no!=0){
$HTTP_SESSION_VARS["user"]=$row['user'];
$HTTP_SESSION_VARS["password"]=$row['password'];
header('Location:crtl.php');
}
else
{
header('Location:login.php?mode=no');
}
?>



This is on the head of every page.

<?
ob_start();
include_once("../includes/dbconn.php");
include("../includes/check.php");
$mode=$_REQUEST['mode'];
$msg=base64_decode($_REQUEST['msg']);


Check.php

<?php
ob_start();
session_start();

$user=$HTTP_SESSION_VARS["user"];
$pass=$HTTP_SESSION_VARS["password"];
header('Location:login.php');
?>


is there anything I can do?

[PFMaBiSmAd]

$HTTP_SESSION_VARS were depreciated long ago (8 years), turned off by default in php5, and completely removed in php6. Use $_SESSION

Each of your header() redirect statements needs an exit; statement after it to prevent the remainder of the code on the page from being executed. All a hacker needs to do is ignore the header() redirect and he can access the content on the page anyway.

The log in code is not escaping the data being put into the SELECT query, so it is possible for a hacker to easily cause the query to match any row in your table without knowing the actual password.

The check.php code does not contain any logic to check what is in the session variables, so it is unlikely that is the actual code. If that is your actual code, you likely have a header() error that is preventing the header() redirect from having any affect, because all visitors (even logged in ones) would be redirected by that code.

[wmguk]

<?
ob_start();
session_start();
include('../includes/dbconn.php');  

$user=$_REQUEST['user'];
$pass=$_REQUEST['pass'];

$sql="select * from admin where username='$user' and password='$pass'";
$result = mysql_query($sql) or die("Query failed : " . mysql_error());
$row=mysql_fetch_assoc($result);

$no=mysql_num_rows($result);

if($no!=0){
$_SESSION["user"]=$row['user'];
$_SESSION["password"]=$row['password'];
header('Location:crtl.php');
exit
}
else
{
header('Location:login.php?mode=no');
exit
}
?>


Hi,

Does this look slightly better?

the check.php is the one in use, what should i do to it to make it secure?

The log in code is not escaping the data being put into the SELECT query, so it is possible for a hacker to easily cause the query to match any row in your table without knowing the actual password.

How could I do this?

[thorpe]

mysql_real_escape_string().

[PFMaBiSmAd]

the check.php is the one in use

Then anyone can visit one of your 'protected' pages and access the content.

You need to find out why the header() redirect is not working AND correct the logic so it tests if the session variable(s) are set (set by a successful log in) and put an exit; statement after the header redirect.

For debugging purposes, add the following two lines of code immediately after the first opening <?php tag on one of your main pages that has the check.php code included on it -

ini_set("display_errors", "1");
error_reporting(E_ALL);

After you find and fix whatever problem is preventing the header from working (for all we know the include() statement is failing and the check.php code is not even involved) you would use code similar to the following to protect a page -

<?php
session_start();
if(!isset($_SESSION["user"])){
    // the current visitor is not logged in
    header('Location: the_url_you_want_to_redirect_to');
    exit;
}