11 replies to this topic

[ajm113]

I'm working on a login system for my website and I dont know if I'm just being stupid, but I dont know why MySQL wont check to make sure the password is equal to the "username". I.E: Lets say I have a table that looks like this: username, password, id.
And someone registers on the website or inserts a row: ['ajm113', '12345', 0].

When they attempt to login they will have no problem of course as long as they have the username correct, but when it comes to the password check, MySQL completely doesn't bother to check it. Someone can type in a password such as "2312asadfasfrer" and login into someone's account! D:

Here is my login script when someone hits the login button.

function HighLevelSecurityString($string)
{
return htmlentities(stripslashes(mysql_real_escape_string($string)));
}


$username =  HighLevelSecurityString($_POST['user']);
$password =  md5(HighLevelSecurityString($_POST['password1']));

				$sql="SELECT * FROM UserList WHERE Username='$username' or email='$username' and Password='$password'";
				$result=mysql_query($sql);
				$row = mysql_fetch_array($result)  or die(mysql_error());

					   // Mysql_num_row is counting table row
						$count=mysql_num_rows($result) or die(mysql_error());

if($count == 1)
{
$_SESSION['username'] = $row['Username'];

header("location:index.php");
}else{
echo"User not found!";
}

As you can tell, very straight forward code. I dont get any errors from MySQL, so I'm not sure how it could possibly return 1 result each time even if the password is completely wrong. What I find strikingly odd is that I'm getting a somewhat reversed effect for when I'm just trying to UPDATE a row. When I do something like. "UPDATE UserList Password='$newPassword' WHERE Username='$username' and Password='$currentPassword'"

[lonewolff]

What if you drop or email='$username' out of the query? It may be an order of operation thing.

[TheUnbeliever]

AND has higher precedence than OR, so your code is effectively "Username='$username' or (email='$username' and Password='$password')"

[WavyVirus]

Also, you should generally be using bind variables

[SimonForsman]

Thus try:
SELECT * FROM UserList WHERE (Username='$username' or email='$username') and Password='$password'

[AllEightUp]

I also suggest dropping real "passwords" and using salted hashes instead. It adds a little more complexity but prevents any login db breaches from being a horrible security mess. Look up "salted passwords" on google, you can find much better descriptions than I can give you here. But in general the idea is that the server doesn't actually know the password, it simply stores a random value and the SHA256+ hash of the combined results. The outline being as follows:

Client logs in and says "I'm user xyz".
Server looks for user "xyz" and finds the salt value (a 64bit or better random number). Server sends the "salt" value back to client.
Client turns the salt into a text string and then does an SHA on "username+saltstring+givenPassword". It sends the resulting 32-64 bytes to the server.
Server compares the "hash" to what it has stored, if equal let them in.

As mentioned, it is a little more complicated but if someone got your user login DB you'd be like, so what, I don't know the actual data used to generate the hash any better than the hackers. So, the worst they could do is set up a longin related fake site and then ask the user to reenter the password and post it directly without the salting. If users fall for the fake site, it's not your problem (mostly), you did the due diligence security and never stored the actual raw data.

I very highly suggest looking into this immediately. It's bad out there and any raw storage as you suggest would be a hackers dream dataset to procure. And there are folks probably reading this making note of your handle for future reference. If you do release anything, they'll look back at this thread and say, eww, easy target.

[rip-off]

Salting is a good idea, but salting is typically handled purely server side. Sending the salt to the client requires an additional round trip to the server, and the end result is the same.

[AllEightUp]

Salting is a good idea, but salting is typically handled purely server side. Sending the salt to the client requires an additional round trip to the server, and the end result is the same.

I'm not entirely certain what you are saying here. If you mean that the client sends the password and the server rebuilds the hash on it's side, that defeats a portion of the security. Part of the reason I (in a C++ app) did it this way was specifically so the password was never sent on the wire and was immediately zero'd in memory after sending. Ok, perhaps I'm overkilling the issue in my code since I exchange public keys, generate stream cypher keys on both sides, exchange those under the public keys, encrypt the stream from that point on yet "STILL" use the public keys for encoding username and the replied hash even in the encrypted stream. Security overkill or useful paranoia?

[Adam_42]

On a website the login will (or at least should) be done over https which should prevent anyone intercepting the password on the wire. You also want to do all the calculations server side so users who have disabled JavaScript can still log in.

The main benefit of the salt is to limit damage if someone steals the whole password file. Without a salt they can use a rainbow table to turn it back into passwords fairly quickly. The salt means they need to brute force each individual password instead.

I found a good explanation of all the details at http://net.tutsplus.com/tutorials/php/understanding-hash-functions-and-keeping-passwords-safe/

[rip-off]

I'm not entirely certain what you are saying here. If you mean that the client sends the password and the server rebuilds the hash on it's side, that defeats a portion of the security.

It depends.

Part of the reason I (in a C++ app) did it this way was specifically so the password was never sent on the wire and was immediately zero'd in memory after sending.

The problem with such a protocol is that the hash of the "password" effectively is the password. That is, the server requires the client knows H(password + salt) to login (call this the "pass token"). While the password is not sent over the wire, the pass token is. If there was a flaw in the cryptography such that I can view the data on the wire, then I can see and replay the pass token to gain access. Note I do not need to brute force the pass token to do this!

Ok, perhaps I'm overkilling the issue in my code since I exchange public keys, generate stream cypher keys on both sides, exchange those under the public keys, encrypt the stream from that point on yet "STILL" use the public keys for encoding username and the replied hash even in the encrypted stream. Security overkill or useful paranoia?

Security is a complex thing. Sometimes trying too hard - e.g. encrypting too much - can result in insecurity. Sometimes adding encryption doesn't make a significant change to the overall security of the protocol. I would be wary of throwing the cryptographic kitchen sink at the problem, in the hopes of improving security. Instead, start with the list of things you want to prevent (packet sniffing passwords, man in the middle, dictionary attacks/brute forcing the password database, etc...), and design¹ a protocol that is immune to these things.

For example, unless you have some out-of-bounds authentication mechanism you didn't mention, the protocol you describe there is vulnerable to MITM. This is exactly the kind of flaw that I was referring to earlier - something that allows a malicious user to obtain the pass token.

Sure, this doesn't directly expose the actual password the user typed (a noble goal). It is also much harder to do a MITM attack than a passive attack. So you've gained some security, but perhaps not as much as you hoped.

It is wise to follow best practise here. The best practise in this case is for the client to send the username/password over HTTPS, and for the server to lookup a salt, compute an expensive hash² of the salt and password, and compare that to the stored value in the database.

 

1. I say design, but this has been done before. Borrow heavily from proven protocols, rather than try to make your own.

2. SHA256 etc are called "secure" hashes, but they designed to be fast to compute digital signatures on large amounts of data. This unfortunately works against the design goals of salting (preventing brute force of a small piece of data). Different hash functions, such as bcrypt(), can offer better resistance to brute force attacks. See this article for a detailed description.

[AllEightUp]

Part of the reason I (in a C++ app) did it this way was specifically so the password was never sent on the wire and was immediately zero'd in memory after sending.

The problem with such a protocol is that the hash of the "password" effectively is the password. That is, the server requires the client knows H(password + salt) to login (call this the "pass token"). While the password is not sent over the wire, the pass token is. If there was a flaw in the cryptography such that I can view the data on the wire, then I can see and replay the pass token to gain access. Note I do not need to brute force the pass token to do this!

My apologies, I revisited the code to make sure about this and I realized that I was actually sending the username+password under encryption. Guess I had already considered what you mention and fixed it but forgot about it. Oops.

As to the rest of it, I completely agree, this is not a web app of course and salsa was chosen for performance reasons and not for the ultimate security.

[ajm113]

Sorry for the few slow feed back (working loong hours at work), I got it working after fixing a few bugs I found in my html and it seems to even work a lot better with the brackets.

Thank you very much guys!