unsubscribe/subscribe/forgotpassword security
Moderator: General Moderators
unsubscribe/subscribe/forgotpassword security
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?
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?
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.
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.
- feyd
- Neighborhood Spidermoddy
- Posts: 31559
- Joined: Mon Mar 29, 2004 3:24 pm
- Location: Bothell, Washington, USA
That isn't a security problem, so that's your choice.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?
Shared servers have their own problems, but those are mostly for the host to worry about.murlopaz wrote:My website is hosted on a shared server, should I be really worried about this?(i use dreamhost)
That doesn't mean it's not possible.. but I'd bet that Dreamhost would void your account if you keep trying.murlopaz wrote:I tried to access somebody else's session on the server, and I was unsuccessful, so that's good news.
Well, that may be something.murlopaz wrote:Also I don't have anything that would counter session fixation.
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:
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.
It is a long shot, but I don't like this:
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.This field is called $confirmation, and it is generated when the user signs up.
...
Should I change $confirmation everytime a user changes his password?
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.
feyd | Please use
delete.php
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.phpCode: 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();
?>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]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;
}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.
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.
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?
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?
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.
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.
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.
should be
Noting the added "; to the line:
Also look into session_regenerate_id() this function might be useful to session fixation.
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();
?>Code: Select all
echo "An email has been sent to you with further instructions.";
$_SESSION=array();
session_destroy();
?>Code: Select all
echo "An email has been sent to you with further instructions.";