tinfish Posted May 20, 2012 Share Posted May 20, 2012 Hi guys, It's my first post here, not looking to leech, I'm simply here to learn and develop my skills and any contributes will be greatly appreciated! Anyways I have made a simple login script, however I would like to make it more secure. However before that, can you please explain to me as to why it is not secure in the first place? A basic explanation so I can understand would be great. Then after that, could you please give help as to how I would make this login code more secure? Thank you very much <?php $rowsfound=false; if (isset($_GET['frmStudentId'])) { // functions to make performQuery() work correctly require_once("dbfunctions.inc.php"); $query = "SELECT dbStudentId, dbStudentName " . " FROM student " . " WHERE dbStudentId = '".$_GET['frmStudentId']."'" . " AND dbPassword = '".$_GET['frmPassword']."'"; $result = performQuery($query); if(count($result) > 0) { $rowsfound=true; // allow login } } // code continues by generating appropriate response ... Quote Link to comment Share on other sites More sharing options...
smoseley Posted May 20, 2012 Share Posted May 20, 2012 For starters, prevent SQL injection: $id = mysql_real_escape_string($_GET['frmStudentId']); $pwd = mysql_real_escape_string($_GET['frmPassword']); $sql = "SELECT dbStudentId, dbStudentName FROM student WHERE dbStudentId = '{$id}' AND dbPassword = '{$pwd}'"; $result = performQuery($query); Of course, you should probably take your passwords to the next level by hashing them with randomly salted MD5 (more secure for your users). $id = mysql_real_escape_string($_GET['frmStudentId']); $pwd = hashup($_GET['frmPassword']); $sql = "SELECT dbStudentId, dbStudentName FROM student WHERE dbStudentId = '{$id}' AND dbPassword = '{$pwd}'"; $result = performQuery($query); function hashup($pwd) { $salt = "5w3we5gwgw52eg55uehgoih34we#G$34gih34g#$wegG34G#4g33weegwgwG$#G3gerG%^H75j&22gwGW"; return md5($pwd.substr($salt, strlen($pwd), 5)); } And then, you could add some protection against bots by limiting the # of attempts... $id = mysql_real_escape_string($_GET['frmStudentId']); $pwd = hashup($_GET['frmPassword']); if (canLogin($id)) { $sql = "SELECT dbStudentId, dbStudentName FROM student WHERE dbStudentId = '{$id}' AND dbPassword = '{$pwd}'"; $result = performQuery($query); if (count($result) > 0) { // successful login } else { registerFailedLogin($id); } } else { echo "Limit 5 login failures per hour."; } function hashup($pwd) { $salt = "5w3we5gwgw52eg55uehgoih34we#G$34gih34g#$wegG34G#4g33weegwgwG$#G3gerG%^H75j&22gwGW"; return md5($pwd.substr($salt, strlen($pwd), 5)); } function registerFailedLogin($uid) { $ip= $_SERVER["REMOTE_ADDR"]; $sql = "INSERT INTO failed_logins (uid, ip_address, timestamp) VALUES ('{$uid}', '{$ip}', NOW())"; mysql_query($sql); } function canLogin($uid) { $ip= $_SERVER["REMOTE_ADDR"]; $sql = "SELECT COUNT(*) as failed FROM failed_logins WHERE uid = '{$uid}' OR ip_address = '{$ip}' AND timestamp > NOW() - INTERVAL 1 HOUR"; $result = mysql_query($sql); $row = mysql_fetch_assoc($result); return $row["failed"] < 5; } Quote Link to comment Share on other sites More sharing options...
tinfish Posted May 20, 2012 Author Share Posted May 20, 2012 Thanks for the swift response. Your help is appreciated! However I have also heard that using $_post as opposed to $_get is also recommended? Although in my eyes it does the same thing? Thanks Quote Link to comment Share on other sites More sharing options...
smoseley Posted May 20, 2012 Share Posted May 20, 2012 Yeah, def use post vs get. I completely missed that! The problem with the get method is that it uses the querystring to pass parameters. So if someone logs in to your site, it'll go to this url: http://www.mysite.com/login.php?frmStudentId=myusername&frmPassword=my_unencrypted_password! That url will get stored in the browser's history, which means anyone can come in and look for http://www.mysite.com/login.php in the browser history to get other people's credentials. I'm sure you can infer the major pitfalls there. 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.