White_Lily Posted November 10, 2015 Share Posted November 10, 2015 Hi, I have been asked by a friend to create a login system for him to use and it requires a password recovery page, and to have all the PHP processing code to be done with AJAX to avoid page 'refreshes', which is fine... However, on the password recovery page, you enter your email address... and nothing happens, it checks the email address is there and that it is a registered email address, but when it comes to updating the database with a new password... it comes up with nothing, no PHP errors, no MySQL errors, and no javascript errors... Here is the update function (no comments on the mysql method... I know it is deprecated, just his choice!)... function mysql_update($table, $columns, $values, $where) { $set = array(); for($up=0;$up<count($columns);$up++){ $set[] = "{$columns[$up]} = '{$values[$up]}'"; } $query = "UPDATE {$table} SET ".implode(",", $set)." WHERE {$where}"; return mysql_query($query); } Quote Link to comment Share on other sites More sharing options...
White_Lily Posted November 10, 2015 Author Share Posted November 10, 2015 Here is the ajax code... jQuery(function($){ $("button#recovery").on("click", function(){ $.post("javascript/ajax/recovery.php", $("form").serializeArray(), function(data){ console.log(data); if(data.success){ $("div#error").addClass("pass").html("<p>" + data.message + "</p><br /><a href='login.php'>Login</a>"); }else{ var errorMsg = ""; for(i=0;i<data.error.length;i++){ errorMsg += "<p>" + data.error[i] + "</p>"; } $("div#error").addClass("fail").html(errorMsg); } }, "json"); }); }); Quote Link to comment Share on other sites More sharing options...
White_Lily Posted November 10, 2015 Author Share Posted November 10, 2015 Here is the PHP code that processing the password recovery... <?php include("../../config.php"); if(isset($_POST)){ $error = array(); if(testStr("empty", $_POST["email"])){ $email = testStr("clean", $_POST["email"]); $check = mysql_select("users", "", "email = '{$email}'", NULL, 1); if(!mysql_num_rows($check)){$error[] = "That email address does not exist.";} }else{$error[] = "Please enter your email address.";} if(count($error) == 0){ $headers = "From: ".config('email/name')." ".config('email/address')."\r\n"; $headers.= "MIME-Version: 1.0\r\n"; $headers.= "Content-Type: text/html; charset=ISO-8859-1\r\n"; $pass = makePassword(15); $salt = salt(); $password = encrypt($pass, $salt); $htmlMessage = " <html> <body> <h2>Password Recovery for Ed's Login System</h2> <p>Hi, you requested a password change. Below you will find your new password.<br /> Once you have logged in, you will be prompted to change it again, this time - remember it!</p> <p><strong>Your New Password:</strong> {$pass}</p> </body> </html> "; if(mail($email, "Password Recovery", $htmlMessage, $headers)){ $update = mysql_update("users", array("password", "salt", "p_prompt"), array($password, $salt, 1), "email = '".$_POST["email"]."'"); if($update && count($error) == 0){ echo json_encode(array("success" => true, "message" => "Password has been changed successfully, check your email for your new password.")); }else{$error[] = "Failed to update password.";} }else{$error[] = "Something went wrong creating your password.";} } if(count($error) > 0){ echo json_encode(array("success" => false,"error" => $error)); } } ?> Quote Link to comment Share on other sites More sharing options...
White_Lily Posted November 10, 2015 Author Share Posted November 10, 2015 Remember the issue is that it doesn't update the database... but returns no errors? Thanks for all your help! Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted November 10, 2015 Share Posted November 10, 2015 (edited) So any visitor of the site can reset anybody's password, and then you send the plaintext password accross the globe so that it can rot in some inbox? Your code is also wide open to SQL injection attacks. Never heard of Bobby Tables? To be honest: Right now, your code makes the site less secure than it would be without any log-in system. So either you get rid of it, or you need to learn basics of security and start from scratch. I know, this sucks. But watching some script kiddie take over the entire server sucks even more. I'm sure your friend agrees. Edited November 10, 2015 by Jacques1 Quote Link to comment Share on other sites More sharing options...
White_Lily Posted November 15, 2015 Author Share Posted November 15, 2015 I know the security risks of this script, however it is just what he wanted. Also, your answer still doesn't explain why the update code does not update the database. You mentioned sanitizing the inputs - which I am doing...: function testStr($type, $string){ switch($type){ case "empty": if(strlen($string) == 0 || $string == NULL){return false;} else{return true;} break; case "clean": $string = strip_tags($string); $string = mysql_real_escape_string($string); return $string; break; } } Now, in any of the code that is posted, is there anything there that would be the cause of the database not being updated? Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted November 15, 2015 Share Posted November 15, 2015 You're not escaping the user input. You do have this weird testStr() function, but you only use it occasionally. Most of the time, you just dump the user input straight into the queries: $update = mysql_update("users", array("password", "salt", "p_prompt"), array($password, $salt, 1), "email = '".$_POST["email"]."'"); ^^^^^^^^^^^^^^^ And I'm fairly sure your friend does not want his server to be compromised. Now, in any of the code that is posted, is there anything there that would be the cause of the database not being updated? You mean besides the total lack of proper input handling? Have you checked the PHP error log? What does it say? If it doesn't say anything, what does it say after you've enabled error reporting and logging? Look, we have no magic debug sense. If you insist on keeping your code, then you will have to narrow down the problem. We cannot do that, because we don't have access to your server, we don't have your code, we don't have your database, we don't even have a problem description. All we know is that the password appearently isn't updated, which could be caused by just about anything. Personally, I'd throw away the code and start from scratch, this time with a proper password reset workflow, proper queries, proper password hashing, proper error handling where you call trigger_error() for every mysql_error(), proper mailing. This sounds like a lot of work, but it's probably faster and definitely more fun than debugging the stuff you currently have. 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.