don4of4 Posted May 16, 2010 Share Posted May 16, 2010 Hi guys, I have this simple login code. The problem is, I need to to display a message when the password is incorrect. I'm clueless how to do this. session_start(); include_once"config.php"; if(isset($_POST['login'])){ $username= trim($_POST['username']); $password = trim($_POST['password']); if($username == NULL OR $password == NULL){ $final_report.="Please complete both fields"; }else{ $check_user_data = mysql_query("SELECT * FROM `members` WHERE `username` = '$username'") or die(mysql_error()); if(mysql_num_rows($check_user_data) == 0){ $final_report.="This username does not exist"; }else{ $get_user_data = mysql_fetch_array($check_user_data); if($get_user_data['password'] == $password){ $start_idsess = $_SESSION['username'] = "".$get_user_data['username'].""; $start_passsess = $_SESSION['password'] = "".$get_user_data['password'].""; $final_report.="<meta http-equiv='Refresh' content='0; URL=members.php'/>"; }}}} I know this is simple, but I'm in a hurry. $1 via paypal to the first to answer Quote Link to comment Share on other sites More sharing options...
public-image Posted May 16, 2010 Share Posted May 16, 2010 Simply do an PHP if if(username[password] != $_post[password]); OR <? $password = $_post[password]; $check_user_data = mysql_query("SELECT * FROM `members` WHERE `username` = '$username'") or die(mysql_error()); $get_user_data = mysql_fetch_array($check_user_data); if($get_user_data[password] != $password){ Echo"Wrong Password!"; } else { Echo"Your password is correct!"; } ?> I think that should work Quote Link to comment Share on other sites More sharing options...
don4of4 Posted May 16, 2010 Author Share Posted May 16, 2010 Simply do an PHP if if(username[password] != $_post[password]); OR <? $password = $_post[password]; $check_user_data = mysql_query("SELECT * FROM `members` WHERE `username` = '$username'") or die(mysql_error()); $get_user_data = mysql_fetch_array($check_user_data); if($get_user_data[password] != $password){ Echo"Wrong Password!"; ?> I think that should work Where should I put that in the original code? Could you give me the whole thing with the message added? Thanks! Quote Link to comment Share on other sites More sharing options...
Psycho Posted May 16, 2010 Share Posted May 16, 2010 I would advise against doing that. Industry standard is to simply state that the username and/or password are incorrect. This does not allow a malicious user to knwo if the username is correct or not. Otherwise, if a malicious user can try random usernames until he gets the error that the password is incorrect. Then he can start trying to hack that user's password. There's several toehr things in that code that are problematic as well from a security standpoint. Are you really storing your passwords without hashing them? Take a look at this. session_start(); include_once"config.php"; if(isset($_POST['login'])) { $username = mysql_real_escape_string(trim($_POST['username'])); $password = mysql_real_escape_string(trim($_POST['password'])); if($username == NULL OR $password == NULL) { $final_report.="Please complete both fields"; } else { $query = "SELECT * FROM `members` WHERE `username` = '$username' AND `password` = '$password'"; $check_user_data = mysql_query($query) or die(mysql_error()); if(mysql_num_rows($check_user_data) == 0) { $final_report.="The username and/or password are incorrect"; } else { $start_idsess = $username = $get_user_data['username']; $start_passsess = $password = $get_user_data['password']; $final_report.= "<meta http-equiv='Refresh' content='0; URL=members.php'/>"; } } } Quote Link to comment Share on other sites More sharing options...
public-image Posted May 16, 2010 Share Posted May 16, 2010 session_start(); include_once"config.php"; if(isset($_POST['login'])){ $username= trim($_POST['username']); $password = trim($_POST['password']); if($username == NULL OR $password == NULL){ $final_report.="Please complete both fields"; }else{ $check_user_data = mysql_query("SELECT * FROM `members` WHERE `username` = '$username'") or die(mysql_error()); if(mysql_num_rows($check_user_data) == 0){ $final_report.="This username does not exist"; }else{ $get_user_data = mysql_fetch_array($check_user_data); if($get_user_data[password] != $password){ // the password is not equal to the one in the database! Echo""; } // Echo above used to tell users if they got the password wrong (FILL THIS IN)! else { Echo""; } // Else, being they did get the right password progress to bellow! $start_idsess = $_SESSION['username'] = "".$get_user_data['username'].""; $start_passsess = $_SESSION['password'] = "".$get_user_data['password'].""; $final_report.="<meta http-equiv='Refresh' content='0; URL=members.php'/>"; }}}} ?> Try that Im not entirly sure if it will work first time but its one step closer If there was an individual value to a certain error do not be specific in that case then the hacker doesnt know what he has got wrong - ive posted the code above anyway although I do agree that there are safer methods out there. Quote Link to comment Share on other sites More sharing options...
don4of4 Posted May 16, 2010 Author Share Posted May 16, 2010 session_start(); include_once"config.php"; if(isset($_POST['login'])){ $username= trim($_POST['username']); $password = trim($_POST['password']); if($username == NULL OR $password == NULL){ $final_report.="Please complete both fields"; }else{ $check_user_data = mysql_query("SELECT * FROM `members` WHERE `username` = '$username'") or die(mysql_error()); if(mysql_num_rows($check_user_data) == 0){ $final_report.="This username does not exist"; }else{ $get_user_data = mysql_fetch_array($check_user_data); if($get_user_data[password] != $password){ // the password is not equal to the one in the database! Echo""; } // Echo above used to tell users if they got the password wrong (FILL THIS IN)! else { Echo""; } // Else, being they did get the right password progress to bellow! $start_idsess = $_SESSION['username'] = "".$get_user_data['username'].""; $start_passsess = $_SESSION['password'] = "".$get_user_data['password'].""; $final_report.="<meta http-equiv='Refresh' content='0; URL=members.php'/>"; }}}} ?> Try that Im not entirly sure if it will work first time but its one step closer If there was an individual value to a certain error do not be specific in that case then the hacker doesnt know what he has got wrong - ive posted the code above anyway although I do agree that there are safer methods out there. Parse error: syntax error, unexpected '}' in /home2/rewardcr/public_html/index.php on line 29 Note: mjdamato's returns an error too. Quote Link to comment Share on other sites More sharing options...
Psycho Posted May 16, 2010 Share Posted May 16, 2010 Shuld be fixed now. Can't test since I don't have your db Quote Link to comment Share on other sites More sharing options...
don4of4 Posted May 16, 2010 Author Share Posted May 16, 2010 Shuld be fixed now. Can't test since I don't have your db WE HAVE A WINNER! At what email address should I be sending your reward? Quote Link to comment Share on other sites More sharing options...
public-image Posted May 16, 2010 Share Posted May 16, 2010 Glad to hear your problems all sorted Quote Link to comment Share on other sites More sharing options...
don4of4 Posted May 16, 2010 Author Share Posted May 16, 2010 Oh, 1 more thing... In the "Your username/pass is incorrect" message, how would I append a link to that? Example: <a href="URL">Recover Password</a> <? session_start(); include_once"config.php"; if(isset($_POST['login'])) { $username = mysql_real_escape_string(trim($_POST['username'])); $password = mysql_real_escape_string(trim($_POST['password'])); if($username == NULL OR $password == NULL) { $final_report.="Please complete both fields"; } else { $query = "SELECT * FROM `members` WHERE `username` = '$username' AND `password` = '$password'"; $check_user_data = mysql_query($query) or die(mysql_error()); if(mysql_num_rows($check_user_data) == 0) { $final_report.="The username and/or password are incorrect."; } else { $start_idsess = $username = $get_user_data['username']; $start_passsess = $password = $get_user_data['password']; $final_report.= "<meta http-equiv='Refresh' content='0; URL=members.php'/>"; } } } if(isset($_SESSION['username']) && isset($_SESSION['password'])){ header("Location: "); } ?> <?php include("includes.php");?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head> <title><?php echo $title ?> | It's time to shop for free!</title> <meta http-equiv="content-type" content="text/html; charset=utf-8" /> <meta name="description" content="Get free stuff from online stores such as Amazon, ASOS, iTunes and more. It takes a few seconds to get started. Register now to start shopping for free." /> <link rel="stylesheet" href="style.css" type="text/css" /> </head> <body> <div id="wrapper"> <div id="header"> <a href="index.php<?php echo $referral_string?>"> <div id="logo"> </div></a> <div id="updates"> <span> </span> </div> <div id="login"> <div id="loginwelcome"> <?php if(!isset($_SESSION['username']) || !isset($_SESSION['password'])){ ?> <?php if($final_report !=""){?> <font color="red"><? echo $final_report;?></font> <?php }else { ?>Welcome Guest, not a member? <a href="register.php<?php echo $referral_string?>"><b>Register Now!</b></a> <?php } ?> </div> <form action="" method="post"> Quote Link to comment Share on other sites More sharing options...
gamblor01 Posted May 16, 2010 Share Posted May 16, 2010 There's several toehr things in that code that are problematic as well from a security standpoint. Are you really storing your passwords without hashing them? Take a look at this. Hashing passwords is better than using them directly, but even better than that is to apply a salt and then hash. This way, 2 identical passwords will not be hashed to the same value (unless the users both have the same salt or there is a collision in your hash function, which is unlikely with good hashes such as md5 or sha1). I use a salt in my code: // Connect to server and select database. mysql_connect("$host", "$username", "$password")or die("cannot connect"); mysql_select_db("$db_name")or die("cannot select DB"); // Define $myusername and $mypassword $myusername=$_POST['myusername']; $mypassword=$_POST['mypassword']; // To protect against MySQL injection $myusername = stripslashes($myusername); $mypassword = stripslashes($mypassword); $myusername = mysql_real_escape_string($myusername); $mypassword = mysql_real_escape_string($mypassword); $sql="SELECT * FROM $tbl_name WHERE username='$myusername'"; $result=mysql_query($sql); // mysql_num_row counts the number of rows returned $count=mysql_num_rows($result); // If result matched $myusername, table row must be 1 row // This means that the user exists in the database if($count==1) { // check the password $row = mysql_fetch_array($result); $storedPassword = $row['password']; $salt = $row['salt']; $encrypted_mypassword=md5($mypassword.$salt); if ($encrypted_mypassword == $storedPassword) { // do whatever you want first and then redirect to login_success.php header("location:login_success.php"); } } When a new user is created I simply generate a salt for them as a random number and then store that in the table with the rest of their information. Quote Link to comment Share on other sites More sharing options...
public-image Posted May 16, 2010 Share Posted May 16, 2010 Yeah that is a safety impliment that could be enforced. Salt or even MD5 Im an MD5 fan but I question its safety myself here is what you put if you would like the experiment $passwordMD5 = md5($password); // Above line is making the password unreadable to hackers trying to acquire user data and its more safe ! Quote Link to comment Share on other sites More sharing options...
Psycho Posted May 16, 2010 Share Posted May 16, 2010 Oh, 1 more thing... In the "Your username/pass is incorrect" message, how would I append a link to that? Example: <a href="URL">Recover Password</a> $final_report.="The username and/or password are incorrect. <a href=\"URL\">Recover Password</a>"; Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.