1

I am setting up a login for a website (perhaps the way I am doing it is just wrong, but that's for another question).

The way I am doing it is like this:

  1. Get username/password from user on login page.
  2. Pass these to my php using post.
  3. In the php, collect the values passed from post.
  4. Compare the password passed to PHP from post, with one taken from a MySQL database.

I can get the database to connect. I can extract the password from the database. I can collect the value from post. But I CANNOT compare these two values - it always says they are not equal, even though echoing them says they are.

To collect the posted values I use:

$posted_username = $_POST['uname'];
$posted_password = $_POST['passwd'];

To query the DB I use:

$result = mysql_query("SELECT passwd FROM usrs where usrname = '".$posted_username."'");

Next, if I include this line:

echo "Posted Username: " . $posted_username . "<br>" . " Posted Password: " . $posted_password . "<br>";

I can see that my posted values have been assigned correctly. I get this output when I run the PHP:

Posted Username: admin Posted Password: admin2

To step through my DB I use:

while ($row = mysql_fetch_array($result, MYSQL_ASSOC))

And inside this while loop I go:

$password_fromdb = $row['passwd'];
echo "Password from db is: " . $password_fromdb . "<br>";
echo "Password from POST is: " . $posted_password . "<br>";
$equal = strcmp($password_fromdb,$posted_password);

The output of the two echo lines gives me this:

Password from db is: admin2 Password from POST is: admin2

So I can see that the two passwords DO match.

However, whether I use strcmp, or whether I use =, == or ====, I ALWAYS get told that my two passwords do not match. I get told they don't match even if they do!

WHAT am I doing wrong? I would appreciate help as I know nothing about PHP, have to do this for a class, and our tutuor has decided he's not going to TEACH us PHP.

3
  • strcmp returns 0, if the two string are equal. Commented Mar 30, 2014 at 8:39
  • Just to give a little bit more info if you have tried both = and ==== these would be incorrect for the following reasons. The first would be an assignment and so would be treated as true (if the assigned value was true) and the latter would be a parse error (unless it was a typo for ===). Triple equals would return true only if the contents match and both values are of the same type i.e. string, which should be the case (and work) for your password comparisons. Commented Mar 30, 2014 at 8:57
  • Hi pebble - yes, you are correct in assuming that "====" was a typo - I meant of course that I used "===", which does not work in this case. Commented Mar 30, 2014 at 13:27

3 Answers 3

4

There's a few (I'd say pretty significant) issues with your code, but your actual issue is looking to be simple.

As you didn't include the code for where you check, I'm going to build off the fact you're using something like if ($equal) { execute your code }:

strcmp returns 0 for being equal. >1 if string1 is greater than string2, and <0 if string2 is greater than string1

See the PHP docs for reference


You can stop reading past this if you're not interested in developing yourself, but I figured I might as well explain a few things you could learn from:

That said however, there are a few issues I would like to raise awareness of, even if this never makes production level. It's a wise experience.

First of all, let me emphasise that what I am about to explain is difficult, and few people get it right - I myself am probably not the most proficient person on this skill either. Later versions of PHP have this covered for us though. But I'll give you the general gist.

In all cases, you will want to hash your passwords. Hashing is a one-way transformation of text that is not humanly readable. Because it is not readable by humans, if your database gets compromised, the passwords are not all publically visible to the intruder, and he will have a harder time getting any data out of it at all.

There is also this thing called salting your passwords, salting is just a way of saying that we add our own string to their password, so it's never completely their password. There's a few ways to do this, but I won't go into detail, because php offers a library that does this for us. Which means we won't have to do anything difficult at all, and can just use their methods. password_hash() and password_verify() - it is only since PHP 5.5, but there are backports to php 5.3.7

You'll have to look up the methods yourself from PHP's official website, as I'm limited to posting only two links. They're found at php.net/password_verify and php.net/password_hash


There is another issue, you're using MySQL, mysql is incredibly old and even deprecated at this point, it is advised you move over to mysqli or even pdo.

Security Issue 2: Your code ($result = mysql_query("SELECT passwd FROM usrs where usrname = '".$posted_username."'");) is vulnerable to MySQL Injection.

MySQL injection is where someone tries to cut off your query, and uses his own. Because you do not properly sanitise user input (never trust the user), The code you're actually executing is not what you may expect it to look like, and may even consist of multiple queries (that could, for example, drop your tables, or even attempt to look up some of those clear-text passwords).

PDO has this covered for us, if you properly make use of the PDO wrapper (located at php.net/pdo), you won't have to worry about this at all. (Well, you do, but it's a whole lot easier to do it right after you get the hang of it).

There also is MySQLi, which is just MySQL_* improved. It allows for OOP programming and is faster. It doesn't sanitise user input for you, you'll have to make use of mysqli_real_escape_string($str), $mysqli->real_escape_string($str), or if you insist on using MySQL_* functions, mysql_real_escape_string($str).


I'll cut off for now, this is a lot of difficult information to process. But I hope someone learnt something from it. And if not, well that's a shame really. As you're really using a few very unsafe ways. Given you may test locally, it's still a good thing to learn good practises.

G'day!

Sign up to request clarification or add additional context in comments.

2 Comments

Zarthus - thanks for your info - a lot of useful stuff there. I'm happy to leave my code as is however because we've just been told that it has to "work" - the tutor didn't say anything about it being secure. In any case, it's not going to be a live site. If I have time once I have it finished I'll implement what you suggested.
@SamuelH hey there Samuel, by no means is it something you should do right away - it's very difficult to grasp them right away, especially if you're new. So I recommend just reading up with it and viewing content others have made prior to implementing it on your own.
1

It might depend on how you're using $equal.

strcmp returns 0 if they are equal (https://secure.php.net/manual/en/function.strcmp.php)

so if you are using if ($equal ) it will fail (because equal was 0)

try to use var_dump($equal) to see what the result is, and try:

var_dump($equal);
if ( $equal === 0 ) {
    echo "It works!";
}

2 Comments

The result of a var_dump on $equal is int(-1). Really not sure WHY my PHP thinks that "admin2" taken from POST and "admin2" taken from my database are NOT equal!
Well believe it or not, I had done something incredibly stupid! I had a space character before the password in my DB. So var_dump($password_fromdb) was returning "String(7)", while var_dump($posted_password)was returning "String(6)". I changed it and everything is working now!
1

I ran into this same (or similar) issue when comparing passwords. strcmp() told me the two strings didn't match, == told me they didn't match... They definitely matched.

My issue is that mcrypt_decrypt() was returning a 32 character string regardless of how long the string actually was. Tre bizarre.

Sample var_dump() output was:

var_dump($password_entered,$password_decrypted_from_database);
string(11) "ThisIsATest"
string(32) "ThisIsATest"

A simple trim() after decryption solved the issue.

Comments