Php Login Issue (Users cannot login and users can use others passwords to access their own accounts)

Question

Ok so here is my issue, Some users cannot login to their accounts. They enter in their password and username properly but it just doesn't seem to work.

and secondly I noticed that a few of the users can log into any account they want with their own password and not the password of the original account.

using MYSQL, and PHP5.

/*Login script*/

if (isset($_POST['Submit'])) {

    $loginUserName = ($_POST['loginUserName']);
    $loginUserName = stripslashes($loginUserName);
    $loginUserName = strip_tags($loginUserName);
    $loginPassWord = ($_POST['loginPassWord']);
    $loginPassWord = stripslashes($loginPassWord);
    $loginPassWord = strip_tags($loginPassWord);
    $loginPassWord = md5($loginPassWord);

    $loginSubmitQuery = "SELECT username,password FROM users WHERE username = '". mysql_real_escape_string($loginUserName ."' and password = '". mysql_real_escape_string($loginPassWord) ."'"; 
    $loginResultQuery = mysql_query($loginSubmitQuery) or die ("Could not find loginUserName and/or loginPassWord");
    $loginResultQuery2 = mysql_fetch_array($loginResultQuery);

     if ($loginResultQuery2) {
        $sql = "SELECT * FROM users WHERE username='".$loginUserName."'";   
        $result = mysql_query($sql)or die('_'.mysql_error());
        while ($row = mysql_fetch_assoc($result)) {
            $_SESSION['player']['id']               = $row['id'];
            $_SESSION['player']['username']         = $row['username'];
            $_SESSION['player']['gamestatus']       = $row['gamestatus'];
            $_SESSION['player']['healthpoints']     = $row['healthpoints'];
            $_SESSION['player']['maxhealthpoints']  = $row['maxhealthpoints'];
            $_SESSION['player']['manapoints']       = $row['manapoints'];
            $_SESSION['player']['maxmanapoints']    = $row['maxmanapoints'];
            $_SESSION['player']['level']            = $row['characterlevel'];
            $_SESSION['player']['strength']         = $row['strength'];
            $_SESSION['player']['defence']          = $row['defence'];
            $_SESSION['player']['monsterid']        = $row['monsterid'];
            $_SESSION['player']['decivers']         = $row['decivers'];
            $_SESSION['player']['experience']       = $row['experience'];
            $_SESSION['player']['nextlevel']        = $row['nextlevel'];
            $_SESSION['player']['inbattle']         = $row['inbattle'];
            $_SESSION['player']['monsterlevel']     = $row['monsterlevel'];
            $_SESSION['player']['monsterid']        = $row['monsterid'];

        }

        echo '<div id="loginaccess"><a href="home.php">[Success Click Here to Login]</a></div>';

    } else {

        echo '<div id="loginaccess">Invalid username or password.</div>';

    }
}
                ?>
            <form id="login" action="" method="POST">
                    <div id="uname" class="formfloat"><p><label for="loginUserName">Username:</label>
                    <input type="text" id="loginUserName" name="loginUserName" value="" class="formfield" tabindex="1" size="15" maxlength="20" /></div></p>
                    <div id="pword" class="formfloat"><p><label for="loginPassWord">Password: </label>
                    <input type="password" id="loginPassWord" name="loginPassWord" value="" class="formfield" tabindex="2" size="15" maxlength="20" /></div></p>
                    <div class="formbreak"></div><br />
                    <input id="loginsubmit" type="submit" value="Login" name="Submit" tabindex="3" >
            </form>
            </div>
        </div>
        <div id="sidebarRight"><!-- sidebarRight div start -->
        </div>

In addition to the answers, you don't need to `SELECT password` in $loginSubmitQuery, it's enough that it's in `WHERE` clause. `SELECT username,password...` doesn't hurt anything, except the performance (a tiny bit); yet it's a bit wasteful to get two values from DB and discard one without using it. — Piskvor left the building, Nov 27 '09 at 08:10

score 4 · Accepted Answer · answered Nov 27 '09 at 07:54

Your SQL query seems to be missing a clause when you're validating the password. WHERE username = '$loginUserName' AND '$loginPassWord' will evaluate to true if the given username exists in the database, and the value of $loginPassWord evaluates to true (because you're not comparing it to anything).

Furthermore, you should be escaping input you get from the user before using it in a database query, using something like mysql_real_escape_string, in order to prevent SQL Injection attacks.

Putting that together, I'd replace your query with something like:

$loginSubmitQuery = "SELECT username, password
  FROM users
  WHERE username = '" . mysql_real_escape_string($loginUserName) . "'
    AND password = '" . mysql_real_escape_string($loginPassWord) . "'";

I was once told not to use mysql_real_escape_string() due to some security flaw. not really sure why. but thank you. also when i stripslashes(), and strip_tags() is it really helping me out? — Jesse, Nov 27 '09 at 08:08
Yes, you should use `mysql_real_escape_string()`. `strip_tags()` just removes HTML and PHP tags so that doesn't do anything to prevent an injection. And `stripslashes()` does the opposite of what you probably think it does, because it turns `\'` into `'`. — Daniel Vandersluis, Nov 27 '09 at 15:20

score 1 · Answer 2 · edited Jun 20 '20 at 09:12

It's also worth noting that you shouldn't store your users' passwords (plaintext) in the database.
It's extremely insecure and exposes your user in bad ways.

To quote the Coding Horror article You're Probably Storing Passwords Incorrectly:

Users collect usernames and passwords like they do Pokemon. It's a sorry state of affairs, but for better or worse, that's the way it is. We, as software developers, are trusted with storing all these usernames and passwords in some sort of database. The minute we store a user's password, we've taken on the responsibility of securing their password, too. Let's say a hacker somehow obtains a list of all our usernames and passwords. Either it was an inside job by someone who had access to the database, or the database was accidentally exposed to the public web. Doesn't matter how. It just happened.

(...)

You might think it's relatively unimportant if someone's forum password is exposed as plain text. After all, what's an attacker going to do with crappy forum credentials? Post angry messages on the user's behalf? But most users tend to re-use the same passwords, probably because they can't remember the two dozen unique usernames and passwords they're forced to have. So if you obtain their forum password, it's likely you also have the password to something a lot more dangerous: their online banking and PayPal.

Those two article cover a little bit of the issue and I suggest you take a read on it.

Never store passwords in a database!

Enough With The Rainbow Tables: What You Need To Know About Secure Password Schemes

For a good compilation of authentication related advice you can also check this StackOverflow question:

The Definitive Guide To Website Authentication (beta)

Edit in response to OP's comment:

Yeah, I'm dumb and only saw that after posting but the comment is still valid.

You're using MD5 hashes but if you look at the linked articles you will see that MD5 alone is not enough and one can easily pick "most" of your passwords back if they have your database.
For a quick test, just throw an MD5 hash into google and you will see that unless your password is really wicked, you will get it back.

For a better approach you'd need the md5hash and a salt for each password, but I'm better pointing you to read the linked article Enough With The Rainbow Tables: What You Need To Know About Secure Password Schemes than trying to explain it here.

It has a simple, easy way to do it right. :)

I'm sure I would do it wrong if I tried to explain it in my own words! :)

i am using the md5 encryption. – Jesse Nov 27 '09 at 18:02 — Jesse, Nov 27 '09 at 18:02

score 0 · Answer 3 · answered Nov 27 '09 at 07:54

0

Did you mean:

SELECT username,password FROM users WHERE username = '$loginUserName' and password = '$loginPassWord'

Instead of:

SELECT username,password FROM users WHERE username = '$loginUserName' and '$loginPassWord'

answered Nov 27 '09 at 07:54

Ivan Krechetov

18,802
8
49
60

And to my surprise it worked. I guess I completely over looked that. – Jesse Nov 27 '09 at 08:19

Php Login Issue (Users cannot login and users can use others passwords to access their own accounts)

3 Answers3

Edit in response to OP's comment: