Security verification

[storm197]

Hi,

I'm building a restricted access site with a Login/Password authentification.

I'm new to PHP and I would like to know if my application is SECURE and Ok to publish to Internet.

Can anyone check my code and tell me if they see security LACK in it ?

In the system, each page has an Include file which verify is the variable $_SESSION['session_allowed']= 1. If not, it redirect to an access denied page.

Thank you very much.

<?php
session_start();

/* For security reasons, if the variable $continue still == 1 at the end, the process
doesn't to check the password */
$continue = 1;
include ("connexion.php");

/* -------------------------------------------------------------------------------------- */
/* User loginname validation */
/* -------------------------------------------------------------------------------------- */

$result = mssql_query("SELECT * FROM tbl_users where login ='$user_login'")
or die ("Requête invalide");

/*Check if the the username exist in the database ?*/
If (mssql_num_rows($result) == 0)
{
$_SESSION['message_login'] = "The account $user_login doesn't exist.";
$continue = 1;
header("location: login.php");
}

/*If the username exist, is the tryout security overpassed ?*/
Elseif (mssql_num_rows($result) == 1)
{
$resultlogin=mssql_fetch_Array($result);
extract($resultlogin);

If ($login_tryout >= 3)
{
$_SESSION['message_login'] = "The account $login have been disabled because of to many login attempts. Please contact Luc Grandchamp.";
$continue = 1;
header("location: login.php");
}

Else
{
$continue=0;
}
}



// --------------------------------------------------------------------------------------
// Password validation
// --------------------------------------------------------------------------------------

if ($continue == 0)
{

$resultpass = mssql_query("SELECT * FROM tbl_users where login ='$user_login' and password ='$login_password'")
or die ("Requête invalide");

//Is the password right ?
// No
If (mssql_num_rows($resultpass) == 0)
{
$_SESSION['message_login'] ="The password for the user $user_login is not right.";
$updated_login_tryout = ++$login_tryout ;

$result_tryout = mssql_query("
UPDATE tbl_users
SET
tbl_users.login_tryout = '$updated_login_tryout'
WHERE tbl_users.login ='$user_login';")
or die ("Requête invalide");

$continue = 1;
header("location: login.php");
}

// Yes
Elseif (mssql_num_rows($resultpass) == 1)
// Define my session's variables
{
$ligne_resultpass = mssql_fetch_array($resultpass);
extract ($ligne_resultpass);
$_SESSION['session_allowed']= 1;
$_SESSION['session_group']= "$user_group";
$_SESSION['session_login_name']= "$login";
$_SESSION['session_id_users']= "$id_users";
$_SESSION['session_user_firstname']= "$user_firstname";
$_SESSION['session_user_lastname']= "$user_lastname";
$_SESSION['message_login']= "";
$_SESSION['user_p_touch']= "$user_p_touch";

// Redirect to the user group directory
if ($user_group ==1)
{
header("location: administrator/menu.php");
}

elseif ($group ==2)
{
header("location: /2");
}

elseif ($group ==3)
{
header("location: /3");
}

elseif ($group ==4)
{
header("location: /4");
}
}
}
?>

 

[sleipnir214]

It appears that your code depends on register_globals to be set to "on". See the PHP online manual section titled "Using Register Globals" for information why this is less secure.


Want the best answers? Ask the best questions!

TANSTAAFL!!

 

[sleipnir214]

Also, you are accepting user input in a SQL query without verifying that input's validity. Although you are using this only in SELECT queries in this particular case, in general a user could enter hostile input that could compromise your database. At the absolute minimum, you should run all user input through mysql_escape_string().


Another comment that is more stylistic than security is that using die() on errors doesn't cause your script to crash very gracefully.




Want the best answers? Ask the best questions!

TANSTAAFL!!

 

[storm197]

Ok, thanks for tose comments.

1- I understand that having the Register_Global =On can be really dangerous. I'll fix that.

2- For the mysql_escape_string(), do you know how I can do this using a MSsql server ?

Thank you.

 

[sleipnir214]

You're right mysql_escape_string() is not an option. Sorry, I wasn't paying enough attention to your code. MSSQL is a little more complicated, as it doesn't use backslashes to escape, for example, single quotes.

I'm not familiar enough any more with the vaguaries of MSSQL to know how to advise you. In general, you need to minimize the damage that input can cause with queries.


Want the best answers? Ask the best questions!

TANSTAAFL!!

 

[storm197]

Ok sleipnir214, I read the Register Globals section on PHP.NET.

Now, I just want to be sure I understand well,

Putting Register Globals = Off means that everytime I want to access a variable that comes from a form, I have to use something like $_POST['username']; $_REQUEST['username']; or $HTTP_POST_VARS['username']; ?

Also, how do I handle the situations where I have to use a link like "details.php?id=$selected_id" ? Is that the same way ?

thanx

 

[sleipnir214]

Data sent on the URL is sent to the server via the GET method. PHP provides $_GET for access to this data. See the PHP online documentation on these variables here.

General Advice:[ul][li]Don't use $_REQUEST. It's as unsafe as having register_globals set to "on".[/li][li]Don't use the "long" variables, for example $HTTP_POST_VARS. Instead use the "short" ones, for example $_POST. The "long" arrays are deprecated and are turned off by default in PHP 5. Also, the "short" variables are are superglobal, where the "long" variables are only global.[/li][li]Use of the input arrays also add a level of autodocumentation to your scripts. Six months from now, you may not remember how the value got into $userid, but $_POST['userid'] is self-explanatory.[/li][/ul]




Want the best answers? Ask the best questions!

TANSTAAFL!!

 

[ericbrunson]

How is $_REQUEST any less secure than $_GET? I don't understand.

 

[sleipnir214]

$_REQUEST contains all the data from $_GET, $_POST, and $_COOKIE. This leads to myriad ways to poison input. Although using $_REQUEST to access GET-method input is probably pretty safe, it would be trivial for someone to introduce information in a URL that will overwrite information in a POST-method input. Remember if two pieces of information have the same variable name, which will overwrite which is dependent on the php.ini setting "variables_order", which may or may not be the same from server and which may not behave as you expect.

If you use the individual superglobals, you also don't have to worry about your own reuse of a variable name in a cookie, for example, to mess with your code which expects data from a form. This makes debugging easier.

And using the individual superglobals maximizes that self-documentation I was talking about earlier. If I use $_POST['foo'] in my code, I'll remember six months from now how that value got into that variable. But all $_REQUEST['foo'] tells me is that it is from some browser data-submission.



Want the best answers? Ask the best questions!

TANSTAAFL!!

 

[Olavxxx]

btw. Is the usergroup some kind of userlevel?
to restrict access to the proper userlevels?

if so, I hope you run some kind of logic in all the pages which need securing, to filter out unauthorized access by users not having the correct userlevel.

read up on this:

http://us2.php.net/manual/en/function.mysql-real-escape-string.php

please read examples!


Olav Alexander Mjelde

http://www.volvo-power.net

Admin & Webmaster