Jump to content
Sign in to follow this  

MySQL Update query...

Recommended Posts



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();
        $set[] = "{$columns[$up]} = '{$values[$up]}'";
    $query = "UPDATE {$table} SET ".implode(",", $set)." WHERE {$where}";
    return mysql_query($query);

Share this post

Link to post
Share on other sites

Here is the ajax code...



                $("button#recovery").on("click", function(){
                    $.post("javascript/ajax/recovery.php", $("form").serializeArray(), function(data){
                            $("div#error").addClass("pass").html("<p>" + data.message + "</p><br /><a href='login.php'>Login</a>");
                            var errorMsg = "";
                                errorMsg += "<p>" + data.error[i] + "</p>";
                    }, "json");

Share this post

Link to post
Share on other sites

Here is the PHP code that processing the password recovery...



    $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 = "
                    <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>
        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));

Share this post

Link to post
Share on other sites

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 by Jacques1

Share this post

Link to post
Share on other sites

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){
        case "empty":
            if(strlen($string) == 0 || $string == NULL){return false;}
            else{return true;}
        case "clean":
            $string = strip_tags($string);
            $string = mysql_real_escape_string($string);
            return $string;


Now, in any of the code that is posted, is there anything there that would be the cause of the database not being updated?

Share this post

Link to post
Share on other sites

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.

Share this post

Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Sign in to follow this  

  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.