Secure Login Logout

[jamarchi]

 

Hi Everybody

 

I've searched and changed this code, but I would like to know if this code is secure, Can someone please help me with that

 

Index.php

<table width=350 border=1 bgcolor=#3399FF>
<?php

//Se inicia la session
session_start();

$username = $_SESSION['username'];
$password = $_SESSION['password'];

//Chequea si hay username y password
if(!$username && !$password){
echo "Bienvenido Visitante! <br> <a href=login.php>Login</a> | <a href=register.php>Register</a>";
}else{
echo "Bienvenido ".$username." (<a href=logout.php>Salir</a>)";
echo "Aqui va la parte protegida ? ";
//echo "<table width=350 border=1 bgcolor=#3399FF>\n";
echo " <tr>\n";
echo "    <td>Esta parte es protegida ?</td>\n";
echo "  </tr>\n";
//echo "</table>\n";
}
?>
</table>

 

Login.php

<?php
session_start();

//Formulario para entrar
function index(){

echo "<form action='?act=login' method='post'>"
    ."Username: <input type='text' name='username' size='30'><br>"
    ."Password: <input type='password' name='password' size='30'><br>"
    ."<input type='submit' value='Login'>"
    ."</form>";    

}

// Esta funcion chequea si la informacion es correcta
function login(){

//Toma la informacion del formulario
$username = $_REQUEST['username'];
$password = $_REQUEST['password'];


//conecta la base de datos
$connect = mysql_connect("localhost", "root", "");
if(!$connect){
die(mysql_error());
}

//Selecciona la base
$select_db = mysql_select_db("base_nombre");
if(!$select_db){
die(mysql_error());
}

//chequea si la informacion es correcta

$result = mysql_query("SELECT * FROM users WHERE username='$username' AND password='$password'");
$row = mysql_fetch_array($result);
$id = $row['id'];

$select_user = mysql_query("SELECT * FROM users WHERE id='$id'");
$row2 = mysql_fetch_array($select_user);
$user = $row2['username'];

if($username != $user){
die("Username incorrecto!");
}


$pass_check = mysql_query("SELECT * FROM users WHERE username='$username' AND id='$id'");
$row3 = mysql_fetch_array($pass_check);
$email = $row3['email'];
$select_pass = mysql_query("SELECT * FROM users WHERE username='$username' AND id='$id' AND email='$email'");
$row4 = mysql_fetch_array($select_pass);
$real_password = $row4['password'];

if($password != $real_password){
die("Password incorrecto!");
}



//Si todo es correcto crea las sesione y permite engresar

session_register("username", $username);
session_register("password", $password);

echo "Bienvenido, ".$username." Para continuar de click aqui en el <a href=index.php>Index</a>";

}

switch($act){

default;
index();
break;

case "login";
login();
break;

}
?>

 

Logout.php

<?php
session_start();

//Aqui se destruye la session
session_destroy();
echo "Usted no esta logueado!, seleccione <a href=index.php>Index</a> o <a href=login.php>Ingresar</a>";

?>

 

REgister.php

<?php

//Muestra el formulario de registro
function register_form(){

$date = date('D, M, Y');
echo "<form action='?act=register' method='post'>"
    ."Username: <input type='text' name='username' size='30'><br>"
    ."Password: <input type='password' name='password' size='30'><br>"
    ."Confirmar password: <input type='password' name='password_conf' size='30'><br>"
    ."Email: <input type='text' name='email' size='30'><br>"
    ."<input type='hidden' name='date' value='$date'>"
    ."<input type='submit' value='Register'>"
    ."</form>";

}

//Registra la informacion del usuario
function register(){

//Connecta la database
$connect = mysql_connect("localhost", "root", "");
if(!$connect){
die(mysql_error());
}

//Selectciona la database
$select_db = mysql_select_db("data_name");
if(!$select_db){
die(mysql_error());
}

//Informacion
$username = $_REQUEST['username'];
$password = $_REQUEST['password'];
$pass_conf = $_REQUEST['password_conf'];
$email = $_REQUEST['email'];
$date = $_REQUEST['date'];

//Apartir de aqui se empieza a chequear la informacion

if(empty($username)){
die("Favor digitar su username!<br>");
}

if(empty($password)){
die("Favor digitar su password!<br>");
}

if(empty($pass_conf)){
die("Favor confirmar su password!<br>");
}

if(empty($email)){
die("Favor digitar su email!");
}

//Chequeamos que el username no este en uso

$user_check = mysql_query("SELECT username FROM users WHERE username='$username'");
$do_user_check = mysql_num_rows($user_check);

//Ahora chequeamos si el email no este en uso

$email_check = mysql_query("SELECT email FROM users WHERE email='$email'");
$do_email_check = mysql_num_rows($email_check);

//Mostramos errores

if($do_user_check > 0){
die("Ese Username ya esta registrado!<br>");
}

if($do_email_check > 0){
die("Ese email ya esta registrado!");
}

//chequeamos que los passwords sean iguales

if($password != $pass_conf){
die("Los password digitados son diferentes!");
}


//Si todo esta bien, se agrega el usuario

$insert = mysql_query("INSERT INTO users (username, password, email) VALUES ('$username', '$password', '$email')");
if(!$insert){
die("Hay un problema: ".mysql_error());
}

echo $username.", ha sido registrado. muchas gracias!<br><a href=?act=login>Ingresar</a> | <a href=index.php>Index</a>";

}

switch($act){

default;
register_form();
break;

case "register";
register();
break;

}

?>

 

Thank you for your help

 

Regards,

[chocopi]

I would say no.

 

1) In your login page and register page you allow for the users to enter whatever they want so they could inject some code into your database. So I would recommend mysql_real_escape_string()

 

2) You are using $_REQUEST which I have heard can be quite dangerous, but I would need someone else to clarify.

 

3) Your session_destroy() will only work if someone loads logout.php, so you could think about using a timer with timestamps, but thats only my opinion.

 

Hope it helps

 

~ Chocopi

[pocobueno1388]

This doesn't really have to do with security, but you are registering sessions the deprecated way.

 

Change this:

session_register("username", $username);

 

To:

$_SESSION['username'] = $username;

 

[DJTim666]

You might wanna think about getting a new logout script. That one will only work if someone loads the logout page by typing; http://www.yoursite.com/logout.php in the address bar. You need a submit button or a link that logs them out instead. I would suggest google.

[jamarchi]

Hi To all

 

Thank you for your answers.....

 

But, can you tell me what i have to do..... i very new in the secure topic in php and i have more than 3 month triying to create a good code.....

 

Thank you...

[cluce]

I agree with the other guy. you are allowing the users to enter whatever they want. you might want to think about using strip_tags() function

[per1os]

Looking at it, I would store the username or userid in the session with an md5 hash of the password. I would also check that information against the database each time the page is loaded. Just to make sure that someone didn't manipulate the data etc.

 

Just my 2 cents.

[jamarchi]

 

What is strip_tags() function?  ???

[DJTim666]

strip_tags() function strips the tags out of a input thing. So if I enter <body> into the field and hit enter, the function would strip the tags and my name is now body instead of <body>. !

[per1os]

www.php.net/strip_tags

 

www.php.net is your best source to answer very easy and simple questions.

[jamarchi]

 

I don't know how to say that.... but, Can some one, please , fix the code ?

 

Thank you

 

If you want contact to me in my msm

[hackerkts]

I don't think here's the place where we do it for you, we're to guide/teach you.

If you re-read the above comments, you should roughly get some ideas.

 

You might want to use preg_replace() on your username and password before it was query into your MySQL.

 

[jamarchi]

 

Yes you right..... sorry for that, but really.... I don't know waht to do or how to do....

[chocopi]

Well I would suggest you read through the tutorial on php.net => LINKY

 

Then maybe look at some of the posts on this forum, as that is how I am learning and you will soon pick things up.

 

Then do some google searches to find php tutorials, which should give you the basis.

 

Lastly, try to learn some of the basic functions of php.

 

---

 

so to use strip tags in your code as other users have mentioned and the mysql_real_escape_string() do this:

 

on register.php

 

replace

 

$username = $_REQUEST['username'];

 

with

 

$username = $_REQUEST['username'];
$username = strip_tags($username);
$username = mysql_real_escape_string($username);

 

And then use md5() on your passwords before your insert them into your database

 

So before

 

$insert = mysql_query("INSERT INTO users (username, password, email) VALUES ('$username', '$password', '$email')");
if(!$insert){
die("Hay un problema: ".mysql_error());
}

 

Add

 

$password = md5($password);

 

I would also suggest adding some validation on you email

 

So after

 

//chequeamos que los passwords sean iguales

if($password != $pass_conf){
die("Los password digitados son diferentes!");
}

 

Add

 

if (!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[_a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,4})$", $email))
{ 
	echo ("Error");
}

 

This will ensure that the user has entered something like: [email protected]

 

 

Thats all I will give you for know, so I suggest you try to learn some basics

 

Hope it helps

 

~ Chocopi

[chrisuk]

I would suggest POST instead of REQUEST to get the username/password from the form

 

Also you may want a session statement at the top of every page so that if user isn't logged in, do not let them view the page. The code for this, for example, could be:

 

<php?
if ($_SESSION['logged_in'] != "yes" {
//take action - either error message or redirect to login page
}
?>

 

frost110 - how would you go about checking the username/password is valid on each page?

[per1os]

You include a file that does a login check.

 

IE:

 

func.gen.php // general functions file

<?php
function verify_user() {
     if (isset($_SESSION['username']) && isset($_SESSION['password'])) {
            $u_data = mysql_fetch_array(mysql_query("SELECT username,password FROM table_name WHERE username = '" . $_SESSION['username'] . "'"));
            if ($u_data['username'] == $_SESSION['username']) {
                     // both should be MD5 hashes
                     if ($_SESSION['password'] == $u_data['password']) {
                                return true;
                     }
            }
     }

      return false;
}

?>

 

index.php

<?php
session_start();

require('inc/func.gen.php'); // must be placed in the inc directory to work.

$valid_user = verify_user();

if (!$valid_user) {
     // show the login form
}else {
    // the user is valid show them the site.
    echo 'Hola ' . $_SESSION['username'] . '!';
}
?>

 

That way you always know if a valid user is viewing the page or not. The code above is sort of half-assed and just used for demonstration.

[chrisuk]

that's really useful, thanks!

 

So just wondering, is this better than checking a session variable (eg 'logged_in') is set at the top of each script?

[cluce]

I am also fairly new to php. I only been messing with it for about a month and found this website very helpful...

 

http://www.w3schools.com/php/default.asp

 

also, I have this book

 

http://ebooks.ebookmall.com/title/sams-teach-yourself-php-mysql-and-apache-all-in-one-meloni-ebooks.htm

[per1os]

that's really useful, thanks!

 

So just wondering, is this better than checking a session variable (eg 'logged_in') is set at the top of each script?

 

It is more secure one way or the other. You could have a variable to check against, but 1 variable that is true/false is a lot easier to spoof than a userid/name and password (hashed of course) combination checking each time against the DB. The load on the DB will not be anymore and will not effect efficiency. Definitely is worth it for security.

[jamarchi]

 

 

Sorry......but until today I could see all the msm....

 

Can some one tell me how can I fix this code in order to hava a good one....

 

Thank you very much