Why prepared statement doesn't work?

Question

I am using this code to check if the user is logged:

    function login_check_admin($mysqli) {
   // Check if all session variables are set
   if(isset($_SESSION['user_id'], $_SESSION['username'], $_SESSION['login_string'])) {

     $user_id = $_SESSION['user_id'];
     $login_string = $_SESSION['login_string'];
     $username = $_SESSION['username'];

     $user_browser = $_SERVER['HTTP_USER_AGENT']; // Get the user-agent string of the user.

     if ($stmt = $mysqli->prepare("SELECT admin_pas FROM admins WHERE admin_id = ? LIMIT 1")) { 
        $stmt->bind_param('i', $user_id); // Bind "$user_id" to parameter.
        $stmt->execute(); // Execute the prepared query.
        $stmt->store_result();

        if($stmt->num_rows == 1) { // If the user exists
           $stmt->bind_result($password); // get variables from result.
           $stmt->fetch();

           $login_check = hash('sha512', $password.$user_browser);

           if($login_check == $login_string) {
              // Logged In!!!!
              return true;
           } else {
              // Not logged in
              return false;
           }
        } else {
            // Not logged in
            return false;
        }
     } else {
        // Not logged in
        return false;
     }
   } else {
     // Not logged in
     return false;
   }
}

Problem is it only works with the last member thats is added to the admins table. As soon as I add another member to the admins table, it returns false when I am logged in with all other members and returns true only when I am logged in with that most recently added member. I changed the code to this and now it works fine. I don't know why prepared statement not work.

function login_check_admin($mysqli) {
   // Check if all session variables are set
   if(isset($_SESSION['user_id'], $_SESSION['username'], $_SESSION['login_string'])) {
     $user_id = $_SESSION['user_id'];
     $login_string = $_SESSION['login_string'];
     $username = $_SESSION['username'];

     $user_browser = $_SERVER['HTTP_USER_AGENT']; // Get the user-agent string of the user.
     if ($result = $mysqli->query("SELECT admin_pas FROM admins WHERE admin_id = ? LIMIT 1")) { 
        if($obj = $result->fetch_object()) { // If the user exists
           $password = $obj->admin_pas; 
           unset($obj);
           $result->close();
           $login_check = hash('sha512', $password.$user_browser);
           if($login_check == $login_string) {
              // Logged In!!!!

              return true;
           } else {
              // Not logged in
              return false;
           }
        } else {
            // Not logged in
            return false;
        }
     } else {
        // Not logged in
        return false;
     }
   } else {
     // Not logged in
     return false;
   }
}

Just by the way, you do not need all the *else* branches. You can simply return `false` at the end of the function. — ComFreek, Aug 03 '13 at 09:19
@YourCommonSense because I changed the code to not use prepared statement and now it works — Zeus Alexander, Aug 03 '13 at 09:27
@YourCommonSense Thank you very much. That was what I really wanted to hear! — Zeus Alexander, Aug 03 '13 at 09:40
Yes, exactly. Now, knowing your assumption was wrong, you can start looking for the real cause. I have edited my answer to give you an idea. — Your Common Sense, Aug 03 '13 at 09:46

score 2 · Answer 1 · edited May 23 '17 at 12:11

2

Here is an improved version of your function

function login_check_admin() {
    if(isset($_SESSION['user_id']) 
       && $_SESSION['user_agent'] == $_SERVER['HTTP_USER_AGENT'])
    {
        return TRUE;
    }
}

To answer the literal question.

A programmer should never judge the code by indirect consequences. Always verify only direct ones. Have an assumption that prepared statement doesn't work? Create a code snippet that contains this statement only and check for the every possible error and verify the state of every variable involved, as well as result of every function and operator. And you will have clear picture - if it really doesn't work, and why if so.

This routine is called "debugging" and can be done by a programmer himself only, not by someone watching the code.

edited May 23 '17 at 12:11

Community

1
1

answered Aug 03 '13 at 09:23

Your Common Sense

156,878
40
214
345

1

Create a *reproduceable* code snippet then, contains prepared statement **only** and prvide an output clearly proving that it doesn't work. – Your Common Sense Aug 03 '13 at 09:33
Thanks for your reply. I did so, the problem is in the prepared statement, it returns a wrong password, I changed the code so it doesn't uses sql prepared statement and it works fine. – Zeus Alexander Aug 04 '13 at 09:00
Also I think your 'improved' version will not work in this situation: Imagine I remove a user from admins table so that he won't have administrative rights any longer, in this case if the user is already logged in from his computer, he will still have admin rights until he logs out, but using my code he will immediately lose admin rights as soon as I remove his name from admins table. Please correct me if i'm wrong. – Zeus Alexander Aug 04 '13 at 09:10
This would work if you cancel user's session along with deletion. – Your Common Sense Aug 04 '13 at 09:15

Why prepared statement doesn't work?

1 Answers1