I'm having issues with possibly md5 encryption

[coaster27]

Hello! I've recently been having trouble with my basic membership website. It is currently being tested with mamp on localhost. So I have two pages, login.php, and register.php. My register.php file is working fine. One of the things about my register file is the password, which is the thing I'm having serious trouble with. Here is the register.php code (Php code only)

$query = "INSERT INTO users (username,password,email) VALUES ('$username',md5(md5('$password'),'$email')";
         		    if (!mysql_query($query)){
         			 die('ERROR: ' . mysql_error . '');
         		    }
         		    echo "You have been registered successfully";

As far as I can see, the register.php file successfully inserts the data into my database, since I can see it through phpMyAdmin.

Now here's my login.php file.

<?php
$checkusername = $_POST['username'];
$checkpassword = $_POST['password'];

if (strlen($checkusername) <= 0){
 echo "<p id='error'>You Need to enter a username!</p>";
}else{
 if (strlen($checkpassword) <= 0){
  echo "<p id='error'>You need to enter a password!</p>";
 }else{
  $query = mysql_query("SELECT * FROM users WHERE username='" . $checkusername ."'");
  if (mysql_num_rows($query) == 1){
   $query = mysql_query("SELECT * FROM users WHERE password='" . md5(md5($checkpassword)) . "'");
   if (mysql_num_rows($query) == 1){
	  echo "Welcome, " . $checkusername . "! You are now logged in!";
 $_SESSION['user'] = $checkusername;
 $_SESSION['pass'] = $checkpassword;
   }else{
 echo "<p id='error'>Wrong Password!</p>";
   }
  }else{
   echo "<p id='error'>Wrong Username!</p>";
  }
 }
}

?>

So, the issue here is when I fill out my php login form, I get a wrong password error. At first, I thought that I wasn't properly connecting to my database table, but I got no error. So I then decided to remove all of the md5 encryption. Without the encryption, my password got entered into the database, and the login file worked. So I think that my problem is the md5(). Now, how do I fix it?

 

I'm sorry if it turns out that the issue was some sort of basic mistake. I'm not the most experienced php coder.

[requinix]

No offense but there's more wrong than just this login issue. Which I'll get to. But there are other things you should know about and fix. In no particular order:

 

1. MD5 is not appropriate for a password. It's much less secure than it was, say, 10 years ago (which is not to say it was secure back then either). And two MD5s is actually worse. Use a better hashing algorithm, like SHA-256 or bcrypt, as well as good principles like salting/HMAC. If you're not sure about this then find a library like PHPass which can handle most of the work for you.

 

2. You're putting $_POST values directly into SQL which opens you up to a vulnerability called SQL injection. That's bad. Putting that aside for a second, the mysql extension and its mysql_* functions are old and shouldn't be used if at all possible. There are better replacements like mysqli and PDO that are faster, easier, and safer to use, and if you use the prepared statements functionality they provide then you don't even have to worry about SQL injection.

 

3. The problem I think you're encountering now is that you check for any user named $checkusername and then you check for any user with the given password. You aren't actually checking if the $checkusername user's password is $checkpassword - any user, any password. More specifically you expect there to be exactly one row for both queries; there's surely only one user with that username, but multiple users could have the same (probably simple) password. You can get the user information and then compare the password, but...

 

4. Presenting two different messages depending if the user exists or if the password is correct is unsafe. As a malicious user with a vendetta against someone I know, I could try to find his account on your site by trying many usernames. Once I get the "Wrong Username" message I'll know I found it and can focus on the password instead. You should only present one message for both cases; if you change your SQL to find a matching user and matching password, at once, then it will save you a lot of repetitious code. Two birds with one stone.

 

5. Although session data is generally safe, you shouldn't put the password in the session. You really don't need to either: if you only keep the username then you can rest assured that the only way the username got in the session is if they logged in correctly. Besides, how often would you need to know the password?

[coaster27]

Thanks for the tips! They really helped, and I figured out my problem, it had nothing to do with the md5(). It was the fact that my users database only had 20 characters, which explains a lot. I also did some of your suggestions. So, thank you

[Psycho]

Well, there's also the fact that the way you hash the password before inserting was not the same as how you hashed when doing the SELECT

 

md5(md5('$password'), '$email')

 

md5(md5($checkpassword))

[shlumph]

I think you have mistyped your INSERT query. I count 4 ('s and only 3 )'s.

[requinix]

Yeah, I think it was a typo in the post. The syntax error would have prevented the insertion entirely and the die() below it would have kicked in.

[coaster27]

The weird thing is that I used that code perfectly without any errors... but thanks for pointing that out