Page 1 of 2

unsubscribe/subscribe/forgotpassword security

Posted: Mon Feb 19, 2007 11:18 am
by murlopaz
Hi everybody. I developed a members area on one of my websites.
I have a couple of security concerns though..

I have a unsubscribe option for a user that is logged in.

When the user click the unsubscribe button, he is sent a email containing a url that will be passed to the delete.php containing a hash of time+username+salt that is saved in a field of the user (in the db). This field is called $confirmation, and it is generated when the user signs up.
IF the user click on the url from the email the delete.php script finds $confirmation field for that particular user and deletes all the information about him as well as kills any sessions that are associated with that particular user.

The same idea is used when a user subscribes as well as when a user forget his password.

Does anybody see any security concerns that I should be aware of?

Posted: Mon Feb 19, 2007 11:32 am
by feyd
On your end, there's nothing to really worry about unless your scripts have holes in them.

Posted: Mon Feb 19, 2007 12:14 pm
by murlopaz
well one thing i noticed is that... if the user wants to change his password, then he could just go directly to the link provided in the email over and over again... and I don't want that...
Should I change $confirmation everytime a user changes his password?


Also, as far as script security goes:

1. I validate input
2. escape output
That's pretty much it... I am also keeping the id number of the user in the session variable. I suppose this is a bad idea.

My website is hosted on a shared server, should I be really worried about this?(i use dreamhost)
I tried to access somebody else's session on the server, and I was unsuccessful, so that's good news.

Also I don't have anything that would counter session fixation.

Posted: Mon Feb 19, 2007 3:07 pm
by feyd
murlopaz wrote:well one thing i noticed is that... if the user wants to change his password, then he could just go directly to the link provided in the email over and over again... and I don't want that...
Should I change $confirmation everytime a user changes his password?
That isn't a security problem, so that's your choice.
murlopaz wrote:My website is hosted on a shared server, should I be really worried about this?(i use dreamhost)
Shared servers have their own problems, but those are mostly for the host to worry about.
murlopaz wrote:I tried to access somebody else's session on the server, and I was unsuccessful, so that's good news.
That doesn't mean it's not possible.. but I'd bet that Dreamhost would void your account if you keep trying.
murlopaz wrote:Also I don't have anything that would counter session fixation.
Well, that may be something.

Posted: Tue Feb 20, 2007 3:08 am
by Mordred
If you want more than an educated guess at your security, post your code!

It is a long shot, but I don't like this:
This field is called $confirmation, and it is generated when the user signs up.
...
Should I change $confirmation everytime a user changes his password?
The password change challenge should be generated each time a user requests a password change and invalidated if he changes his password or after a short (1 day?) timeout period.

Your setup does not mitigate the risk of intrusion enough. Imagine an attack when someone can read your database, and assume the login credentials themselves are well protected ([plug="shameless"]Do you use double-salted hashes?[/plug]). The attacker still has a strong entry point - he can use the $confirmation tokens to change the passwords of ALL users or delete their accounts.

Posted: Wed Feb 21, 2007 9:01 am
by murlopaz
feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


This is the script that emails the user a link that would delete him from the db.

unsubscribe.php

Code: Select all

<?
/************************GET unique Confirmation ID from DB*********************/
$connection=mysql_connect($hostname,$username,$password) OR DIE('DB Error:1 ' . mysql_error());
mysql_select_db("$dbName", $connection) or die('DB Error:2 ' . mysql_error());
$sql="SELECT confirmation 
          FROM users 
	  WHERE userID=".quote_smart($_SESSION["user"]);

//execute query
if (!($result = @ mysql_query ($sql, $connection)))
    die('DB Error:3 ' . mysql_error());
$row=@mysql_fetch_array($result);
$confirmation=$row["confirmation"];

//get sms_email value
$sql="SELECT sms_email from sms_emails WHERE userID=".quote_smart($_SESSION["user"]);

//execute query
if (!($result = @ mysql_query ($sql, $connection)))
    die('DB Error:3 ' . mysql_error());

$row=@mysql_fetch_array($result);
$sms_email=$row["sms_email"];
/************************GET unique Confirmation ID from DB*********************/

/************************UNSUBSCRIBE EMAIL BEGINS HERE**********************/
$from="abc";
$header = "";
$footer = "";
$subject = "ACCOUNT CONFIRMATION";
$body = "Hello. Please click on  the following link to unsubscribe your account 
		 from the system: blahblahblah/delete.php?id=".$confirmation;	   
//***change link later on to reflect the correct path***
$to=$sms_email;
$from="abc";
mail("$to", "$subject","$header\n  $body\n $footer\n", "from:$from");
/************************UNSUBSCRIBE EMAIL ENDS HERE************************/

echo "An email has been sent to you with further instructions. 


$_SESSION=array();
session_destroy();
?>
delete.php

Code: Select all

<?
$id=$_GET["id"];
//validate this !!!
/************************GET unique Confirmation ID from DB*********************/
$connection=mysql_connect($hostname,$username,$password) OR DIE('DB Error:1 ' . mysql_error());
mysql_select_db("$dbName", $connection) or die('DB Error:2 ' . mysql_error());
$sql="SELECT userID from users WHERE confirmation=".quote_smart($id);

//execute query
if (!($result = @ mysql_query ($sql, $connection)))
    die('DB Error:3 ' . mysql_error());
$row=@mysql_fetch_array($result);
$count=@mysql_num_rows($result);
if ($count==1)
{
	$id=$row["userID"];
	//deletes user's entry from users table
	$sql="DELETE from users WHERE userID=".quote_smart($id);
	if (!($result = @ mysql_query ($sql, $connection)))
		die('DB Error:3 ' . mysql_error());
	//deletes user's entry from user_role table
	$sql="DELETE from user_role WHERE userID=".quote_smart($id);
	if (!($result = @ mysql_query ($sql, $connection)))
		die('DB Error:3 ' . mysql_error());
	//deletes user's entry from sms_phones table
	$sql="DELETE from sms_phones WHERE userID=".quote_smart($id);
	if (!($result = @ mysql_query ($sql, $connection)))
		die('DB Error:3 ' . mysql_error());
	//deletes user's entry from sms_emails table
	$sql="DELETE from sms_emails WHERE userID=".quote_smart($id);
	if (!($result = @ mysql_query ($sql, $connection)))
		die('DB Error:3 ' . mysql_error());
/************************GET unique Confirmation ID from DB*********************/
	
	echo "Your have been successfully unsubscribed.";
}else if ($count==0)
{
    echo "Invalid ID.";
}else
{
    echo "Unexpected error, please contact the administrator";
}
?>

feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]

Posted: Wed Feb 21, 2007 9:17 am
by Mordred
Show us quote_smart() as well.

Posted: Wed Feb 21, 2007 9:20 am
by murlopaz

Code: Select all

function quote_smart($value) {
   if (get_magic_quotes_gpc()) {
       $value = stripslashes($value);
   }

   if (!is_numeric($value) || $value[0] == '0') {
       $value = "'" . mysql_real_escape_string($value) . "'";
   }
   return $value;
}

Posted: Wed Feb 21, 2007 9:38 am
by Mordred
This part seems secure, there's just the question of sms_email, which should be cleaned of \r and \n in order to avoid email header injection. Are you doing this?

I'd put the quotes in the SQL statements and mysql_real_escape_string() in all ocasions - there's no problem to assign '123' to a INT field, but this is your coding style, not mine ;)

There is a possible problem (not security related) - if you quote_smart a value that hasn't come from $_GET, $_POST etc. and magic_quotes are enabled, you would blindly strip the slashes off it, which is not correct.

Posted: Wed Feb 21, 2007 9:49 am
by murlopaz
Thanks Mordred for your help.

sms_email is validated + escaped when the user subscribes...

There is one more concern I have. I use the confirmation for unsubscribe, subscription confirmation as well as when somebody forgets their password.

This approach seems clumsy to me...

Also can anybody illustrate how a hacker can use session fixation to impersonate a user?

I am planning on using https. This should exclude the possibility of the hacker capturing anything related to the session...

Are there other ways that a hacker can take advantage session fixation?

Posted: Wed Feb 21, 2007 10:01 am
by Mordred
As I said before, the confirmation token has to be generated ONLY when the user requests deletion or password change, and have a validity period. Since it is temporary, there's no problem to use it for both purposes.

About session hijacking/fixation and https, I suggest you find some reference papers and read the basics.
Session fixation != session hijacking, and https will (generally) NOT help against either.

Posted: Wed Feb 21, 2007 10:28 am
by murlopaz
what would be a good confirmation token?
hash of (username+time+salt).

Is that good enough?

Posted: Wed Feb 21, 2007 10:41 am
by Mordred
A random string of 8+ alphanumeric characters. Do not md5 a random int and cut 8 chars off it, it's not the same ;)

Posted: Wed Feb 21, 2007 10:45 am
by murlopaz
well there is a slight possibility that two users would get the same confirmation number...

Posted: Wed Feb 21, 2007 12:49 pm
by Z3RO21
Also remember not to output mysql_error() to the end user. This can reveal relevant information to the user that could be used against you.

Also in your code for unsubscribe.php you have a code error.

Code: Select all

echo "An email has been sent to you with further instructions.


$_SESSION=array();
session_destroy();
?>
should be

Code: Select all

echo "An email has been sent to you with further instructions.";


$_SESSION=array();
session_destroy();
?>
Noting the added "; to the line:

Code: Select all

echo "An email has been sent to you with further instructions.";
Also look into session_regenerate_id() this function might be useful to session fixation.