Poor login script

[Nixon]

Hi All,

I currently have to be honest a poor login script. I'm not sure on how to improve it so I am looking for some help and also some critique on my method. Any ideas at all please don't hesitate to post.

Thanks.

<?php

session_start();

include("includes/connectdb.php");

addslashes($tbusername);
addslashes($tbpassword);

addslashes($ckusername);
addslashes($ckpassword);

$count=mysql_query("SELECT COUNT(*) AS count FROM memberinfo WHERE username='$tbusername' AND password='$tbpassword'");
$row=mysql_fetch_assoc($count);

if ($row[count]=="1") 

{ 

session_register('seusername');
session_register('sepassword');
session_register('log');
session_register('thissession');

$log='Y';
$seusername=$tbusername;
$sepassword=$tbpassword;
$thissession=1;

if ($remember=="checked") 

{

setcookie("ckusername", $seusername, time()+604800); 
setcookie("ckpassword", $sepassword, time()+604800); 
setcookie("log", "Y", time()+604800);

}

print "<script> location.href='membersarea.php' </script>"; 

}

else { print "<script> location.href='login.php?login=bad' </script>"; die(); }

?>

[mrhappiness]

assuming it works 😉

• don't use session_register but use $_SESSION instead

• $tbpassword comes from your login form?
  you don't really store a plaintext password in a clientside cookie?

• i don't see a login based on the values stored in the cookie

• use $row['count'] and not $row[count]

• javascript based redirection is bad
  why don't you use [man]header()[/man]?

• addslashes isn't good either, what if your hoster activates magic_quotes_gpc?
  try something like

  if (get_magic_quotes_gpc())
  {
  // remove all escaping characters if present
    $tbusername = stripslashes($tbusername);
    $tbpassword = stripslashes($tbpassword);
  }
  $tbusername = mysql_escape_string($tbusername);
  $tbpassword = mysql_escape_string($tbpassword);

  if you can use [man]mysql_real_escape_string[/man] use it instead of mysql_escape_string

• how do you check if a user is logged_in?
  why do you store username and password in your session?

• why not retrieving the user's id and store it in your session?
  if the id is present the user is logged in, if not he is not

[Nixon]

Thanks ill try and add all those improvements 🙁 might need help with some of them.

[ShawnK]

Here's a suggestion for checking to see if a user is logged in:

<?php 

session_start();

if(session_is_registered('seusername'))
{ 
header("Location: [url]http://site.com/index.php[/url]");
} else {
header( "Location: [url]http://site.com/login.php[/url]" ); 
}

?>

pretty simple...just include this code into the IF login is successful part of your page.

If you really want to you could connect to your db again and test the user and pass...might slow down your script ALOT though.

[RonR]

<?php

print "<script> location.href='membersarea.php' </script>"; 
?>

If I'm gonna surf directly to membersarea.php will I have access?

[ShawnK]

You will if that page looks something like this...

<?php  

session_start(); 

if(session_is_registered('seusername')) 
{  

/*
 * PAGE CONTENT(IF LOGGED IN)
 */
} else { 
header( "Location: login.php"); // if not logged in the re-direct to login page.
} 

?>

[planetsim]

Why are you programming as if register_globals were off that is bad.

use $POST $GET dont use session_register, session_is_registered etc

use $SESSION to destroy this session use unset($SESSION['session_name']);

This way if you try to use the session variable which is

$seusername

With that like it is its possible to be overwritten in a script and break the sessions.