seems to work, but is it safe...?

[kaiman]

Hi again,

 

I'm back with more newbie PHP questions 

 

I am curious about the security of the login script below and how I could do some simple things to make it stronger. I have been reading about securing sessions in particular

session_regenerate_id();

but I am wondering how to use this in the script below?

 

Currently I have a login script that looks like this:

 

// connects to server and selects database.
include ("dbconnect.inc.php");

// table name
$tbl_name="registered_members";

// protect against mysql injection
function cleanString($string){
    htmlentities(mysql_real_escape_string($string));
    return $string;
}

// username and password sent from login form
// $username=$_POST['username'];
$username = cleanString($_POST['username']);
$pass = sha1($_POST['pass']);

$sql="SELECT * FROM $tbl_name WHERE username='$username' and password='$pass'";
$result=mysql_query($sql);

// mysql_num_row counts the table row
$count=mysql_num_rows($result);

// if result matched $username and $pass, table row must be 1 row
if($count==1){

// register $username, $password and redirect to member page
session_start();
$_SESSION['username'] = $username;
$_SESSION['pass'] = $pass;
header( "Location: http://www.domain.com/members/" );
}
else {
echo "Incorrect Username or Password";
exit ;
}

 

At the top of each members page I have this:

 

<?php
session_start();
if(!isset($_SESSION['username'])){
header("Location: http://www.domain.com/login/" );
exit;
}
?>

 

and for the logoff I have this:

 

<?php
session_start();
$_SESSION = array();
session_unset();
session_destroy();
?>

 

Also, I am planning on using HTTPS to encrypt the pages with SSL. My questions are these:

 

1. Will this cleanString function I have suffice for the fact that magic_quotes_gpc is enabled on the server I am using and I have no access to the php.ini file?

 

2. Is there an easy way to implement the

session_regenerate_id();

function in this script?

 

3. Are there any other glaring deficiencies that I should address?

 

4. Should I use other $_SESSION functions like $ip addresses or user $id to keep users in their sessions and prevent attacks?

 

Any help or constructive feedback would be appreciated.

 

Thanks!

 

kaiman

[cags]

With the fact that you know you have magic_quotes enabled on the server, I'd imagine you would end up with double escaped strings. It's my belief that if you have magic_quotes enabled you should call stripslashes on the input before calling mysql_real_escape_string,

[kaiman]

Yep, the php manual confirms that, thanks for the tip! Any other suggestions?

 

Also, do I need to use those functions on the password field?

[cags]

Assuming your password field is hashed then strictly speaking no (unless you use a hash method that can contain characters that require escaping), I always tend to find myself doing it anyway, even if just for the sake of posterity.

[kaiman]

I am using sha1, which I don't believe contains characters that need escaping, but correct me if I am wrong...

[xtopolis]

I'm not sure where you would add session_regenerate_id() to your scripts; I feel you only wish to add it because you read about it, not that you understand its use.  This function is used to keep the users session data intact while transferring it to another (PHP)SESS_ID.  Examples cited are situations such as when a user receives a privilege pro/de-motion.  While it can be used on every page request, it is not necessarily recommended.

 

Also, you may wish to limit your characters allowed for a username and filter out the unwanted ones with a regular expression.  You understand that you should escape things with mysql_real_escape_string and htmlentities, but again, it seems you are using them because you think you should, not that you understand their uses  (I'm referring to using htmlentities(), it may be appropriate, but it is uncommon from my experiences).  Typically usernames are alpha-numeric, though I have seen ones in the more popular forums that follow the \W allowed characters.

[kaiman]

I'm not sure where you would add session_regenerate_id() to your scripts; I feel you only wish to add it because you read about it, not that you understand its use.  This function is used to keep the users session data intact while transferring it to another (PHP)SESS_ID.  Examples cited are situations such as when a user receives a privilege pro/de-motion.  While it can be used on every page request, it is not necessarily recommended.

 

Also, you may wish to limit your characters allowed for a username and filter out the unwanted ones with a regular expression.  You understand that you should escape things with mysql_real_escape_string and htmlentities, but again, it seems you are using them because you think you should, not that you understand their uses  (I'm referring to using htmlentities(), it may be appropriate, but it is uncommon from my experiences).  Typically usernames are alpha-numeric, though I have seen ones in the more popular forums that follow the \W allowed characters.

 

Um, best practices are best practices regardless of whether I understand all the details or not. I don't understand regex that much at all, for example, but I know that if used correctly it will help filter out different things such as unwanted characters or invalid email addresses. I am not a programmer by trade, so some of the finer details escape me, but that doesn't mean that I don't understand their uses...

 

BTW good point on limiting characters for the username, I will add that function to my scripts.

[trq]

Um, best practices are best practices regardless of whether I understand all the details or not. I don't understand regex that much at all, for example, but I know that if used correctly it will help filter out different things such as unwanted characters or invalid email addresses. I am not a programmer by trade, so some of the finer details escape me, but that doesn't mean that I don't understand their uses...

 

BTW good point on limiting characters for the username, I will add that function to my scripts.

 

Point is, its not necessarily a best practice to regenerate a sessions id on each request. It adds overhead for no real benefit. The session id only really need to be regenerated when a users access is escalated.

[kaiman]

Thanks for clarifying that thorpe... without sound condescending.