Logging in users, avoiding cancelled user accounts?

Facts:

1. Users on our system are allowed to login via their email address and pasword.
2. Email addresses are the unique identifier, but if an accoutn is canceled, another person can signup with the same email address. The previous record remains in our system though.
3. If a cancelled user tries to login, we should not allow him access.

Now, when checking if the record exists when logging in a user, is it correct to do the following?

$rs=$DBobject->execute_query("select * from users where `email`='".$email."' and `password`='".passwordMD5($password)."' and status!='cancelled'");
if($DBobject->my_num_rows($rs)>0)
{
    // login the user
}
else
{
    // check if account has been cancelled 
    $rs_cancelled=$DBobject->execute_query("select id from users where `email`='".$email."' and `password`='".passwordMD5($password)."' and status='cancelled'");
    if($DBobject->my_num_rows($rs_cancelled)>0)
    {
        return "Your account has been cancelled";
    }
    else
    {
        return "Invalid email/password";
    }
}

I'm particularly concerned about the fact that we're using the pas开发者_运维百科sword to uniquely identify the record (in $rs_cancelled) - can that be used for nefarious purposes? How can the process above be subverted?

Unless the variables that go into your queries are sanitized somewhere we don't get to see, it can be subverted in minutes by any kid with an SQL injection tool.

Always escape variables before putting them into queries. Since you are using some database wrapper (a custom one?), I can't tell you the exact way to do this. Generally (e.g. if using PDO) it involves parameterized prepared statements.

Also, this code is kind of wasteful. What about this:

// WARNING -- WARNING -- HERE PROBABLY BE SQL INJECTIONS
$rs=$DBobject->execute_query(
    "select * from users where `email`='".$email."' and password`='".passwordMD5($password)."'");

// Is there such a user?
if($DBobject->my_num_rows($rs) == 0) {
    return "Invalid email/password";
}

// Now fetch the record from $rs -- I don't know how your code does this,
// but let's assume:
$row = $rs->getRow();

// Now check if the user is cancelled
if ($row['status'] == 'cancelled') {
    return "Your account has been cancelled";
} 

// Login the user, since the account exists and is not cancelled

This way you only have to hit the db once for a successful login.

You are not using only the password for authentication. Technically, another user shoulf not be able to create an account using an already used and cancelled email address, because he is not able to confirm the email address. I would solve the problem by adding a UNIQUE constraint on the email field - that would eliminate your potential problem/s

If you use email to identify the user, and if some user account is cancelled, you should either delete the user record in order to allow user to sign up again using same email OR the email should not be allowed to signup again.