[Deansatch]

I have wrote a simple login script and was wondering if it is secure and if not, how insecure is it.

The Login Script:

PHP Code:

require("../config/config.php");

$connection = mysql_connect("$host", "$usr", "$pwd")or die("cannot connect");
mysql_select_db("$db")or die("cannot select DB");

$username = mysql_real_escape_string(strip_tags($_POST['username']));
$password = mysql_real_escape_string(strip_tags($_POST['password']));

$query = mysql_query("SELECT * FROM users WHERE username ='$username' and password = '$password'");
$count = mysql_num_rows($query);

if($count==1){

session_register('username');
session_register('password');
$_SESSION['username'] = $username;s 
$_SESSION['password'] = $password;
header("location:welcome.php");
}

else {
header("location:error.php");


} 


The login checking at the top of each file:

PHP Code:

require('../config/config.php');

$username = $_SESSION['username'];
$password = $_SESSION['password'];
if (!session_is_registered("username") and !session_is_registered("password"))
{
header("location:error.php");exit;
}
$connection = mysql_connect($host,$usr,$pwd);
$query = mysql_db_query($db, "SELECT * from users where username = '$username' and password = '$password'", $connection);
$row = mysql_fetch_assoc($query);
$u = $row["username"];
$p = $row["password"];
if($u != $username or $p != $password ){
header("location:error.php");exit;} 


[nfriedly]

I don't see any glaring security holes in your script, but I have a couple of suggestions:

1) don't store passwords as plain text in your database. This is to protect your users should your database ever be compromised. Instead, store them as a hash of some sort. md5() is amazingly simple, and crypt() is only slightly more difficult to implement.
http://us2.php.net/md5
http://us2.php.net/crypt

2) you don't really need to store their password in the session - I doubt you'll be needing it again. Especially if it's encrypted.

3) location headers are *supposed* to have the full http://domain.com/whatever.php, not just the whatever.php. However, prettymuch every browser out there can handle it, so this is mostly a moot point.
http://www.w3.org/Protocols/rfc2616/....html#sec14.30

[Deansatch]

If I don't store the username and password in the session, how can I check the login status on each page to make sure they are logged in? i.e. so that someone can't just type the url of an edit page and be logged in from there since they didn't have to go through the login part?

[nfriedly]

Do store the username, just not the password. If the username is stored in the session, you can safely assume that their password was already checked and they are logged in.

[Deansatch]

ah! Ok. Thanks a lot. If I md5 the passwords, how would I be able to send out password reminders? Can php decode them again?

[Keyne]

Originally Posted by Deansatch

ah! Ok. Thanks a lot. If I md5 the passwords, how would I be able to send out password reminders? Can php decode them again?

You have to generate other password and change, MD5 is one-way encrypt

About your script, I have some sugestions, look:

PHP Code:

<?php
# THIS CAN STAY IN config.php
session_start();

require("../config/config.php");

# THIS CAN STAY IN config.php
$connection = mysql_connect($host, $usr, $pwd)or die("cannot connect");
mysql_select_db($db)or die("cannot select DB");

$username = mysql_real_escape_string($_POST['username']);
$password = mysql_real_escape_string($_POST['password']);


$query = mysql_query("SELECT * FROM users WHERE username ='$username' and password = '$password'");
$count = mysql_num_rows($query);

if($count == 1) {

    $r = mysql_fetch_assoc($query);

    $_SESSION['id'] = $r['id']; 
    $_SESSION['username'] = $r['username'];
    
    header("location: http://.../welcome.php");

}

else {
    header("location: http://.../error.php");
}  
?>

The login check:

PHP Code:

<?php
# THIS CAN STAY IN config.php
session_start();

require('../config/config.php');

if (empty($_SESSION['username']) || empty($_SESSION['id'])) {
    header("location: http://.../error.php");
    exit();
}
?>

Do a test!