j.smith1981 Posted December 16, 2009 Share Posted December 16, 2009 Hi peeps! Just wanted to gather some advice from you guys. I just wanted to make sure I was heading the alright way in making sure my php code is secure for a php & mysql 5 login, the script is as below (it works at the moment aswell!). Was taken with the advice of this tutorial: http://www.phpeasystep.com/workshopview.php?id=6 Ok enough waffle here's the script: It consists of a login.php (html form), checklogin.php (logs user in or rejects), login_success.php (what they see if they are logged in) Here's the form 'login.php': <html> <body> <h1>My Login</h1> <br> <form name="login" id="login" method="post" action="checklogin.php"> <tt>Username:<input name="uname" id="uname" type="text" /></tt> <br> <tt>Password:<input name="passwd" id="passwd" type="password" /></tt> <br><br> <input type="submit" name="submit" id="submit" value="Login"> <input type="reset" name="reset" id="reset" value="Clear"> </form> </body> </html> Here's the 'db_config.php': <?php ob_start(); $db_host = 'localhost'; $db_uname = '*****'; $db_passwd = '*****'; $db_name = 'test'; $tbl_name = 'login'; // Connect to database server (MYSQL in our case!) mysql_connect($db_host,$db_uname,$db_passwd)or die('Cannot connect to db_server'); mysql_select_db($db_name)or die('Cannot connect to database'); ?> Here's the 'checklogin.php': <html> <head> </head> <body> <?php //require_once("db_config.php") or die("Couldn't load myfile"); include 'db_config.php'; // Gather users form entry and apply to variables $username = $_POST['uname']; $password = $_POST['passwd']; // Strip slashes from user input (preventing SQL injection) $username = stripslashes($username); $password = stripslashes($password); // Remove escaped strings (due to SQL injection prevention!) $username = mysql_real_escape_string($username); $password = mysql_real_escape_string($password); $encrypted_pass = md5($password); // Save SQL syntax query to a variable // NEEDS TO USE REGEXP! // OLD QUERY, MIGHT SAVE ON MEMORY! //$sql = "SELECT * FROM $tbl_name WHERE username='$username' AND password='$encrypted_pass'"; // SQL only retrieves id, ukm no and username of the user logged in, store this in a cookie? $sql = "SELECT id, ukm, username FROM $tbl_name WHERE username='$username' AND password='$encrypted_pass'"; // Perform SQL query set it to a variable $result = mysql_query($sql); // Make variable count and then count the number of rows returned by performing query $count = mysql_num_rows($result); // Only if it equals 1 peform the login script! if($count==1) { session_register("username"); session_register("password"); setcookie("user_name",$username); header("location:login_success.php"); //DISABLE TO MAKE SURE VARIABLES ARE BEING CARRIED THROUGH! echo $result; echo '<tt>You will be logged in!</tt>'; } else { echo '<tt>Wrong username or password, please try again</tt>'; ob_end_flush(); { ?> <br><br> <form name="login" id="login" method="post" action="checklogin.php"> <tt>Username:<input name="uname" id="uname" type="text" /></tt> <br> <tt>Password:<input name="passwd" id="passwd" type="password" /></tt> <br><br> <input type="submit" name="submit" id="submit" value="Login"> <input type="reset" name="reset" id="reset" value="Clear"> </form> <?php }} ?> </body> </html> Finally the results page 'login_success.php': <?php session_start(); if(!session_is_registered(username)){ header("location:login.php"); } ?> <html> <body> <tt>You are logged in as: </tt><?php echo $_COOKIE["user_name"]; ?> <br><br> <tt>Please enter your Daily Sales Analysis below on the below form:</tt> </body> </html> As I think I may have suggested, any advice is wonderfully appreciated, this is the first time I have ever completed some kind of PHP & MySQL login system so there will be problems I suspect, but if you could give me some advice. I have thought about this critically, but some advice on how to maybe complete a logout bit would be appreciated like, from that earlier link it suggests to use this: // Put this code in first line of web page. <? session_start(); session_destroy(); ?> How would I implement the 'session_destroy();' function? Also I am wanting to put in data relating to the user, ie this is meant to save data from a user like a sales analysis system, but how would I best do this? So the user can input data, I will take a backup of this and try my own way and just compare your feedback and apply it. Thanks in advance for any help, I have had much appreciated help off this forum in the past and know someones going to come back with some helpful comments, please dont think I will take this to heart, as again this is the first time I have ever done this. Kind regards and I really look forward to any replies, the sooner the better in all honesty. Thanks again, Jeremy. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/ Share on other sites More sharing options...
Adam Posted December 16, 2009 Share Posted December 16, 2009 I just wanted to make sure I was heading the alright way in making sure my php code is secure for a php & mysql 5 login First of all, considering you mentioned PHP5, you're using deprecated session methods (session_register, session_is_registered) as of PHP5.3 - if you're wanting to 'modernize' the code this would be the first place I'd start. I think that link you've been learning from has been teaching you old tricks! In the way of security, echoing out any form of user input without proper filtering leaves you open to XSS attacks: echo $_COOKIE["user_name"]; The login process itself seems secure enough though, except I think this is a bit superfluous (and doesn't prevent SQL injections as it says): // Strip slashes from user input (preventing SQL injection) $username = stripslashes($username); $password = stripslashes($password); Looks like that's catering for "magic_quotes" - again deprecated as of PHP5.3. I have thought about this critically, but some advice on how to maybe complete a logout bit would be appreciated like, from that earlier link it suggests to use this: // Put this code in first line of web page. <? session_start(); session_destroy(); ?> How would I implement the 'session_destroy();' function? That's pretty much it really - not mentioning the 'short_open_tags'. Take a look at the manual for a better description and examples: http://php.net/manual/en/function.session-destroy.php As I mentioned before though, that tutorial you've been following seems out of date. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-978584 Share on other sites More sharing options...
Psycho Posted December 16, 2009 Share Posted December 16, 2009 You are duplicating your form. It appears you have the initial form and then, if login fails, you duplicate the form. There's no need to do that. Also, I would suggest that you do not mix the logic (PHP) and the output (HTML). Do all the loigic and then do the presentation. In other words do not put <HTML> on the page until you've done the logic. I see some minor issue, such as you are saving the escaped string for username to session instead of the "plain" username. Plus, there is some error handling missing. Here is a rewrite with some changes I would make. This is not tested. So there may be some minor typos, but the logic should be sound. login.php (this is the main page that will always be called. It will validate the form, if needed. The user does not call the form directly) <?php //Variable to track any errors $error = ''; //Check if form was posted if (isset($_POST)) { //Validate required data was entered if (!isset($_POST['uname']) || trim($_POST['uname'])=='' || !isset($_POST['pword']) || trim($_POST['pword'])=='') { //Missing data $error = "Required data was not entered."; } else { //Data was submitted, execute db query and validation include ('db_config.php'); //Assign value for submited data (used to repopulate form) $username = trim(stripslashes($_POST['uname'])); $password = trim(stripslashes($_POST['pword'])); //Assign values for query use $sqlUsername = mysql_real_escape_string($username); $sqlPassword = mysql_real_escape_string(md5($password)); //Create and run query $sql = "SELECT id FROM $tbl_name WHERE username='$sqlUsername' AND password='$sqlPassword'"; $result = mysql_query($sql); //Check for errors if(!$result) { //Error running query $error = "An unknown error occured, please try again."; } else if(count($result)!=1) { //No match found $error = "Wrong username or password, please try again."; } else { //Login successful session_register("username"); session_register("password"); setcookie("user_name",$username); header("location:login_success.php"); } } } //Form was not posted or there were errors, show form include('login_form.php'); ?> login_form.php (This is only called from the above script) html> <body> <h1>My Login</h1> <br> <tt><?php echo $error; ?></tt> <br> <form name="login" id="login" method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>"> <tt>Username:<input name="uname" id="uname" type="text" value="<?php echo $username; ?>" /></tt> <br> <tt>Password:<input name="pword" id="pword" type="password" /></tt> <br><br> <input type="submit" name="submit" id="submit" value="Login"> <input type="reset" name="reset" id="reset" value="Clear"> </form> </body> </html> Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-978601 Share on other sites More sharing options...
j.smith1981 Posted December 17, 2009 Author Share Posted December 17, 2009 Ahhh ace! Thanks ever so much for your feedback and will use this as a reference to make that one even better and post it as per the solutions youve explained above. I will contact the author of that tutorial and see if they have either an updated one, and if not (excuse punctuation) see if they would update to my version, with your amendments. But nevertheless thanks ever so much, I knew someone (in this case 2) people would reply lol. Kind regards as always, Jeremy. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-979011 Share on other sites More sharing options...
j.smith1981 Posted December 18, 2009 Author Share Posted December 18, 2009 I have the following now then: login_form.php contains (the user enters for the first time yea?): <html> <head> <title>FTP Admin Account Login : jeremysmith.me.uk</title> </head> <body> <h1>FTP Admin Account Login : jeremysmith.me.uk</h1> <tt><b>Please login below: </b></tt><br><br> <form name="login" id="login" method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>"> <tt>Username:<input name="uname" id="uname" type="text" value="<?php echo $username; ?>" /></tt> <br> <tt>Password:<input name="pword" id="pword" type="password" /></tt> <br><br> <input type="submit" name="submit" id="submit" value="Login"> <input type="reset" name="reset" id="reset" value="Clear"> </form> </body> </html> and then login.php looks like: <?php $error = ''; // Makes a variable error assigned a null value if (isset($_POST)) // Creates an if statement if form has been processed! { if(!isset($_POST['uname']) || trim($_POST['uname']) == '' || !isset($_POST['pword']) == '') // If any input fields are empty { $error = 'Required dat a was not entered.'; // Fill in error response! TRY MAKING IT APPRECIATE USERNAME AND PASSWORDS ARE EITHER CORRECT SORT OF! } else // Else do the below (if all have been entered!): { include('db_config.php'); // Connect to database // Assign value for submitted data (used to populate form): $uname = trim(stripslashes($_POST['uname'])); $passwd = trim(stripslashes($_POST['pword'])); // Assign login for SQL query, more economical for SQL and PHP5! $sqlUsername = mysql_real_escape_string($uname); $sqlPassword = mysql_real_escape_string(md5($passwd)); // Sets SQL variable and inserts actual SQL query for user authentication! $sql = "SELECT id FROM $tbl_name WHERE username='$sqlUsername' AND password='$sqlPassword'"; $result = mysql_query($sql); if(!$result) // For some reason an unknown error { $error = 'An unknown error occured, try re-entering your login and submitting again.'; } else if (count($result)!=1) { $error 'You entered the wrong username or password!'; } else { session_register("uname"); session_register("passwd"); setcookie("user_name",$username); header("location:login_success.php"); } } } // Include the login form! include('login_form.php'); ?> Then if their login works, it brings them to that sucess page yea? But this isnt working I have found, I dont see how the login_form.php is meant to be transferring somehow to login.php (I did actually put this in as the server index in a .htaccess like file), but it came up blank and realised the above statement by yourself. Can you help me again please? Does look like a very professional bit of code so I thank you loads for helping me. Any feedback is greatly appreciated! Regards, Jeremy. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-980016 Share on other sites More sharing options...
abazoskib Posted December 18, 2009 Share Posted December 18, 2009 I cleaned it up a bit. A few things worth noting: if($POST) will ALWAYS return true, so its unnecessary. Not tested. <?php $error = ''; // Makes a variable error assigned a null value if(!isset($_POST['uname']) || trim($_POST['uname']) == '' || !isset($_POST['pword']) || trim($_POST['pword']) == '') { // If any input fields are empty $error = 'Required data was not entered.'; // Fill in error response! TRY MAKING IT APPRECIATE USERNAME AND PASSWORDS ARE EITHER CORRECT SORT OF! } else { // Else do the below (if all have been entered!): include_once('db_config.php'); // Connect to database // Assign login for SQL query, more economical for SQL and PHP5! $sqlUsername = mysql_real_escape_string(trim($_POST['uname'])); $sqlPassword = mysql_real_escape_string(md5(trim($_POST['pword']))); // Sets SQL variable and inserts actual SQL query for user authentication! $sql = "SELECT id FROM $tbl_name WHERE username='$sqlUsername' AND password='$sqlPassword'"; $result = mysql_query($sql); if(!$result) { // For some reason an unknown error $error = 'An unknown error occured, try re-entering your login and submitting again.'; } else if (mysql_num_rows($result)!=1) { $error 'You entered the wrong username or password!'; } else { $SESSION["uname"]=$uname; $SESSION["passwd"]=$passwd; setcookie("user_name",$username); header("location:login_success.php"); } } // Include the login form! include('login_form.php'); ?> Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-980175 Share on other sites More sharing options...
Psycho Posted December 18, 2009 Share Posted December 18, 2009 login_form.php contains (the user enters for the first time yea?): I cleaned it up a bit. A few things worth noting: if($POST) will ALWAYS return true, so its unnecessary. You've implemented my code backwards. The user should ALWAYS be directed to the logic (i.e. PHP) page login.php. The HTML page login_form.php is called by the logic when it's the first visit to that page or if validation fails. So, in that case, POST is not always set. On the first visit to the page, the logic will bypass all of the validation code and directly load the form. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-980263 Share on other sites More sharing options...
j.smith1981 Posted January 13, 2010 Author Share Posted January 13, 2010 Sorry for the delay in getting back to you. It appears that the form seems to log me in all the time, cant work out why this is maybe you can help me please? Here's what I have (sorry to be a pain): login.php: <?php //Variable to track any errors $error = ''; //Check if form was posted if (isset($_POST)) { //Validate required data was entered if (!isset($_POST['uname']) || trim($_POST['uname'])=='' || !isset($_POST['passwd']) || trim($_POST['passwd'])=='') { //Missing data $error = "Required data was not entered."; } else { //Data was submitted, execute db query and validation include ('db_config.php'); //Assign value for submited data (used to repopulate form) $username = trim(stripslashes($_POST['uname'])); $password = trim(stripslashes($_POST['passwd'])); //Assign values for query use $sqlUsername = mysql_real_escape_string($username); $sqlPassword = mysql_real_escape_string(md5($password)); //Create and run query $sql = "SELECT username FROM $tbl_name WHERE username='$sqlUsername' AND passwd='$sqlPassword'"; $result = mysql_query($sql); //Check for errors if(!$result) { //Error running query $error = "An unknown error occured, please try again."; } else if(count($result)!=1) { //No match found $error = "Wrong username or password, please try again."; } else { //Login successful session_register("username"); session_register("password"); //setcookie("user_name",$username); header("location:login_success.php"); } } } //Form was not posted or there were errors, show form include('login_form.php'); ?> Which calls the form: login_form.php if the user hasnt logged in properly yet: <html> <body> <h1>My Login</h1> <br> <tt><?php echo $error; ?></tt> <br> <form name="login" id="login" method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>"> <tt>Username:<input name="uname" id="uname" type="text" value="<?php echo $username; ?>" /></tt> <br> <tt>Password:<input name="passwd" id="passwd" type="password" /></tt> <br><br> <input type="submit" name="submit" id="submit" value="Login"> <input type="reset" name="reset" id="reset" value="Clear"> </form> </body> </html> Apologies if I have got this wrong. Thanks for your reply in advance. Jeremy Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-994401 Share on other sites More sharing options...
Psycho Posted January 13, 2010 Share Posted January 13, 2010 Ah, yes... Change this else if(count($result)!=1) To this else if(mysql_num_rows($result)!=1) Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-994413 Share on other sites More sharing options...
j.smith1981 Posted January 13, 2010 Author Share Posted January 13, 2010 When you mentioned this: In the way of security, echoing out any form of user input without proper filtering leaves you open to XSS attacks: echo $_COOKIE["user_name"]; How would I then go about displaying the users username in the login_success.php page? Thanks again in advance, will have a look at session destroy aswell. You have been of a great help as always, really prefer this forum as the people are so kind on here! Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-994461 Share on other sites More sharing options...
Psycho Posted January 13, 2010 Share Posted January 13, 2010 It is simply a matter of escaping the user input based upon the usage. If you are using the input in a mysql query, then you want to use mysql_real_escape_string(). If you are going to use as text within an html page then you can use htmlentities() to escape an user input that might otherwise be interpreted as HTML code. For example, if the user has a username of "<BOB>" the browser would interpret that as an opening tag and could disrupt the output or not display it visually (although there are more malicious uses of such things). The htmlentities() function will convert the <> to the appropriate codes so they are not interpreted as HTML. E.g. "<Bob>" However, if repopulating user input into a form field you should only need to escape the quote marks. But each usage is different. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-994555 Share on other sites More sharing options...
j.smith1981 Posted January 13, 2010 Author Share Posted January 13, 2010 Alright can you check this for me? <?php session_start(); if(!session_is_registered(username)) { header("location:login.php"); } ?> <html> <head> <title>jeremysmith.me.uk : Example Members Area </title> </head> <body> <tt><b>You have been successfully logged in!</b></tt> <br><br> <tt><b>Please change your login details below:</b></tt> <br><br> <form name="amend" id="amend" method="post" action="amend_login.php"> <tt><b>Username:</b></tt> <input type='text' id='uname' name='uname' value='<?php echo $_COOKIE['uname']; ?>' readonly /> <br><br> <tt><b>Password:</b></tt> <input type='text' id='passwd' name='passwd' value='' /> <br><br> <input type="submit" id="submit" name="submit" value="Change" /> <input type="reset" id="reset" name="reset" value="Clear" /> </form> <br> <form name="logout" id="logout" method="POST" action="logout.php"> <input type="submit" id="logout" name="logout" value="Logout" /> </form> </body> </html> Have I done that in the most secure way? All I want is to be able to allow a user to be able to amend their password. I am really learning allot, will be reffering to this code as a sort of library for myself. I really thank you allot for helping me. Also the logout.php looks like: <?php //start the session //session_start(); //check to make sure the session variable is registered if(session_is_registered('username')){ //session variable is registered, the user is ready to logout session_unset(); session_destroy(); } else { //the session variable isn't registered, the user shouldn't even be on this page header("location:login.php"); } ?> But when I go back into the login_success.php page it says I am still logged in, is there something I am missing here, gone over it about 8 times now and cant see where I am going wrong here. Thanks ever so much again, Jeremy. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-994561 Share on other sites More sharing options...
j.smith1981 Posted January 13, 2010 Author Share Posted January 13, 2010 Ahhh sorry I got the logout.php script working! Dont know why I didnt see this, the more I go over it though the more it makes sense. <?php session_start(); if(!session_is_registered(username)) { header("location:login.php"); } //session variable is registered, the user is ready to logout session_unset(); session_destroy(); header("location:login.php"); ?> Just needed another look I think lol. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-994563 Share on other sites More sharing options...
j.smith1981 Posted January 15, 2010 Author Share Posted January 15, 2010 So basically when the user has entered their details correctly. The MySQL query comes back with 1 row. I then start the php session with: session_start(); Is this right? I then go to pass the variables I want (like path for the web directory their in?) and their username as feedback to them yea? Like so: $_SESSION['session_var'] = $prev_variable (like I have used as per above $sql_username;) or which ever applies to what variable the username is in yea? I then on the success page put: $username = $_SESSION['session_var'] and then: echo "$username"; Of course using double quotes yea? I think I have finally got the hang of this though, seems to work perfectly for what I want, so I thank you loads. Just as a general guidance for myself, whats your tips on making this more secure and also, just thinking what if some users have disabled cookies in their browser, whats your best idea for getting around this? I cant believe I can now do successful php logins, quite impressed with what I have managed to do, as a result of getting tips from you, really learning allot from this forum and will keep coming back when I have questions. Thanks you've been of a marvelous help! Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-995484 Share on other sites More sharing options...
j.smith1981 Posted February 5, 2010 Author Share Posted February 5, 2010 It is simply a matter of escaping the user input based upon the usage. If you are using the input in a mysql query, then you want to use mysql_real_escape_string(). If you are going to use as text within an html page then you can use htmlentities() to escape an user input that might otherwise be interpreted as HTML code. For example, if the user has a username of "<BOB>" the browser would interpret that as an opening tag and could disrupt the output or not display it visually (although there are more malicious uses of such things). The htmlentities() function will convert the <> to the appropriate codes so they are not interpreted as HTML. E.g. "<Bob>" However, if repopulating user input into a form field you should only need to escape the quote marks. But each usage is different. Ahh makes perfect sense now, sorry dont know why I didnt catch that before, ah well least I know now what you mean! I have now seen this on a few examples I have been working from. This has to be easily the best forum to ask for advice on php issues, you people really have taught me allot. Because of what you have done, I am adding this into my theories for my work in Smarty HTML (obviously adding another tier in my employed work aswell), this post is a question about an element of a system, I am doing but also what I will be using within my work in PHP. Serious kind regards to both of you for helping me, its much appreciated! Thanks again, Jeremy. Quote Link to comment https://forums.phpfreaks.com/topic/185363-help-required-on-php-mysql-login-security-check-script/#findComment-1007462 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.