asked on

PHP login script

I currently have the following code in login.php. Every page that I want users to be logged in for I simply include login.php. I've read discussion boards and I have a feeling that this code is not very secure. Can you please help me make this code more secure. Eventually I hope to partner this script with a mysql database which will store the usernames and passwords. I also want to add different levels of users (ie not logged in, student level, teacher level, administrator level). Please help!

<?
session_start(); // start session.

?>
<html>
<head>
<title>Login</title>
</head>
<body>
<?
if(!isset($log_username) & !isset($log_password)) {
?>
<form action="<?=$PHP_SELF?><?if($QUERY_STRING){ echo"?". $QUERY_STRING;}?>" method="POST">
Members only. Please login to access this document.<br>
Username: <input type="text" name="log_username"><br>
Password: <input type="password" name="log_password"><br>
<input type="submit" value="Login">
</form>

</body>
</html>
<?
exit();
}

session_register("log_username");
session_register("log_password"); // register username and password as session variables.

if($log_username == "freddo_frogs" & $log_password == "are_yummy") {
$valid_user = 1;
} else {
$valid_user = 0;
}

// If the username exists and pass is correct, don't pop up the login code again.
// If info can't be found or verified....

if (!($valid_user))
{
session_unset(); // Unset session variables.
session_destroy(); // End Session we created earlier.

?>
<form action="<?=$PHP_SELF?><?if($QUERY_STRING){ echo"?". $QUERY_STRING;}?>" method="POST">
Incorrect login information, please try again. You must login to access this document.<br>
Username: <input type="text" name="log_username"><br>
Password: <input type="password" name="log_password"><br>
<input type="submit" value="Login">
</form>
</body>
</html>
<?
exit();
}
?>

jalalmegadeth

if($log_username == "freddo_frogs" & $log_password == "are_yummy")

it will be more secure if you store those values in the database if you use any

brad_tho

ASKER

Thanks, I'll do that! I feel as though other aspects are not secure.
Also, any suggestions for my other additions?

Cornelia Yoder

1. You should use session variables as a security measure. When a user logs in, you will check the login and password against the values in your database, and then set a session variable. In each script accessed by that user, check that the session variable is set. This prevents someone from accessing your system without going through the login process.

2. If the user will be accessing different pages after logging in, you should use a randomly generated session id number each time he logs in. At login, randomly generate a large number and store it in the database as part of his data. Then pass that number as part of the data passed in every form or url that the user accesses, and check it in every script against his data. By creating a new number at each login, you help prevent someone who is logged in from accessing data belonging to someone else's account.

brad_tho

ASKER

Thanks that sounds great, though I'm not really familliar with session variables. Can you give me an idea to get started with? Thanks!

Cornelia Yoder

Sure, start here:

http://us3.php.net/manual/en/ref.session.php

You will simply register a session variable when the user login is verified, and then in each script check to see if that variable is still registered.

session_start();

check the login and password against the database, if NOT ok
session_destroy();
and exit.

if ok
session_register("loginok");

then in every script
if ( ! session_is_registered("loginok"))
session_destroy();
and exit

otherwise continue normally.

Latest versions of php are switching to the use of $_SESSION global array instead of the session functions, but either way works. There are examples in the online manual at the above link.

Member_2_3684445

Next to that it might be usefull to register the session also into a database. This way you will be able to determin how long the session has been idle. If the session is Idle to long you can logout the user prompting for the password again. Next you can also check if someone else is allready logged in using these credentials, wich might be something you dont want.

Also use md5() encryption to store your password in the database, next validate it against the md5() encrypted password entered by the user. ie.

if( isset($_SESSION['username']) || isset($_POST['username']) ){
$username = (isset($_SESSION['username'])) ? $_SESSION['username'] : $_POST['username'];
}else{
$username = false;
}

if( isset($_SESSION['password']) || isset($_POST['password']) ){
if(isset($_SESSION['password']) ){
$password = $_SESSION['password'];
}else{
$password = md5($_POST['password'];
}
}

$sql = 'select username, password
from users_table
where username = "'.$username.'"';
if($result = mysql_query($sql)){
$row = mysql_fetch_array($result);

// Do all the checks //
$show_form = false;
}else{
// some sql error?
$show_form = true;
}

if($show_form == true){
// some login form//
}else{
// Set or Reset session vars //
// Define something to see if session is logged in //
}
}

dasmaer

yodercm said >> At login, randomly generate a large number and store it in the database as part of his data.

You should also do this to stop anyone from giving their username details to another person. If person A logs in as "bob" and then person B also logs in as "bob", then the number will change... and therefore invalidate person A's credentials. This way only 1 "bob" can be logged into the system at any one time.

D

brad_tho

ASKER

Thanks for all that, is it safe against injection attacks?
And how do I differentiate between different levels of users?

dasmaer

You can tell user levels by setting a new session variable and then checking it where you need it. And to protect against insertion attacks you should use preg_replace() to remove the characters: ; | & < > ( ) [ ] $ on any input.

Cornelia Yoder

To guard against Injection, you need to do the following:

For EVERY input into your program, including even hidden variables in forms, you pass the value through the php function htmlentities( ). (Not preg functions please)

For example, in the script that processes a form sending values for login and password, use these statements for each input (such as password in this example):

$password = $_POST["password"];
$password = htmlentities($password, ENT_QUOTES);

You can do this in one statement as

$password = htmlentities($_POST["password"], ENT_QUOTES);

http://us3.php.net/manual/en/function.htmlentities.php

If a value is numeric, also use a function to ensure it is a valid number:

$num = $_POST["numberin"];
$num = floor($num);

or any other arithmetic function that will not change the value of a valid numeric input but WILL convert any non-numeric value to zero.

Cornelia Yoder

What do you mean about "different levels of users"? What levels?

ASKER CERTIFIED SOLUTION

gemdeals395

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

gemdeals395

Thanks for the grade brad! Hey on the javascript function when I checked it in IE it was putting out the warning if the _GET values were missing for username & password so in case you want to use it here is the fixed script ;)

<script language="javascript">
function UserPass() {
            var credentials = window.location.search;
            var extract = new Array();
            var username = '';
            var password = '';
            if (credentials) {
                  credentials = credentials.replace('?', '');
                  extract = credentials.split("&");
                  username = extract[0].split("username=");
                  password = extract[1].split("password=");
                  username = username.slice(1,username.length);
                  password = password.slice(1,password.length);
                  document.login.username.value = username;
                  document.login.password.value = password;
            }
            return true;
}
</script>

Enjoy ;)

brad_tho

ASKER

Thanks but im not going to use javascript... i have a thing about client-side scripting...

gemdeals395

Ok, Didnt know if you wanted it or not since I had it there since the person I was helping it wanted that feature ;)

Enjoy ;)

Cornelia Yoder

Well, even though you didn't think the session variables and sql injection information was worth a few points, use them anyway and good luck to you.

brad_tho

ASKER

Sorry yodercm, your information was extremely useful and I will use it, but unfortunaly, I dont know how to give you both points!

Cornelia Yoder

It's ok, you got the main thing you needed from gemdeals, but for the future, when you want to split points among several responders, there is a button at the bottom that allows you to do that. Enjoy the holiday season, and DO be sure and use the anti-injection stuff!

brad_tho

ASKER

I will thanks!