username and password validation in php mysql

Question

What I want to be able to do is: When a user enters their username and password in the form on the index.html page, if they match what is in the DB, then they get sent to the next page; userlogin.php. If their username or password is incorrect then they are asked to re-enter their details on the index.html page, and displaying an error like, "Your username is Incorrect" or "Your password is Incorrect" above the form text box. I can paste this code if required.

Can I change this text font color as well, to red for example?

This is the code I currently have for the userlogin.php page

<?php
mysql_connect("Server", "root", "Gen") or die("Couldn't select database.");
mysql_select_db("generator") or die("Couldn't select database.");

$username = $_POST['username'];
$password = $_POST['password'];

$sql = "SELECT * FROM users WHERE Username = '$username' AND Password = '$password' ";
$result = mysql_query($sql) or die(mysql_error());
$numrows = mysql_num_rows($result);
if($numrows > 0)
   {
    echo 'Your in';
   }
else
   {
    echo 'Your not in';
   }
   ?>

Please, read about sql injection http://php.net/manual/en/security.database.sql-injection.php — corretge, Sep 01 '11 at 08:26
Jokes aside, you should learn about [SQL injection](http://en.wikipedia.org/wiki/SQL_injection#Technical_Implementations) and [how to avoid it](http://en.wikipedia.org/wiki/SQL_injection#Mitigation) — Zecc, Sep 01 '11 at 08:32

score 15 · Answer 1 · edited May 23 '17 at 12:06

There as sooo many things wrong with this code:

1- you have an SQL injection hole.

If I enter ' or 1=1 LIMIT 1 -- as a username, I will always get access, no matter what.
Change your code into.

$username = mysql_real_escape_string($_POST['username']); 
$password = mysql_real_escape_string($_POST['password']);

See: How does the SQL injection from the "Bobby Tables" XKCD comic work?

2- you are storing the password in the clear
This is a huge no no. Combined with the SQL-injection hole, it will take a hacker 5 minutes to get a list of all usernames and passwords on your site.

Store the password as a salted hash.

I like to use the username as the salt.

You store the password hash using:

INSERT INTO users (username, passhash) 
VALUES ('$username', SHA2(CONCAT('$password','$username'),512))

And you test the user credentials using:

SELECT * FROM users 
WHERE username = '$username' AND 
passhash = SHA2(CONCAT('$password','$username'),512)

See: Secure hash and salt for PHP passwords
And: What is "salt" when relating to MYSQL sha1?

BTW, use SHA2 with a 512 keylength, SHA1 is no longer secure, and MD5 is even more broken.

3- A login can only ever match against 1 user
This code:

if($numrows > 0)

Makes no sense, if you get 2 rows out of the database, that's a clear sign someone has hacked your system. The test should be:

if($numrows > 1) { //send email to sysadmin that my site has been hacked }
else if ($numrows = 0) { echo "wrong username or password" }
else { echo "welcome dr. Falken" }

4- Don't die if there's an error, call a routine to restart the connection or something

This code:

$result = mysql_query($sql) or die(mysql_error());

Is fine in testing, but in production you should do something like

$result = mysql_query($sql);
if ($result) {
  //do the deed
} else {
  //call error recovery routine
}

The error recovery routine should reconnect to the server, log a error in the logbook. Is the error cannot be fixed, it should send an email to the sysadmin and only then die the server.

a side question, how safe is this: foreach ( $_GET as $id => $val ) $_GET[$id] = mysql_real_escape_string ( $val ) ; — Bojan Kogoj, Sep 01 '11 at 08:45
@Bojan, there's no problem using the $_GET in your code, there's only a problem when you insert the $_GET into an SQL or HTML string. In the first case you need to worry about SQL-injection, in the latter you need to worry about XSS. Both problems are covered extensivly on SO, just search for `[php] [xss]` or `[php] [mysql] SQL injection` on stackoverflow. — Johan, Sep 01 '11 at 08:54
Well i was given that by a friend and he said that that should keep it safe (along with POST one), but i'm in doubts. I know about sql injection problem, website i once made got hacked that way.. forgot to filter one input — Bojan Kogoj, Sep 01 '11 at 09:07
@Bojan, if you have a question, by all means ask a question on SO, why are you using a comment to ask me a specific question without context. — Johan, Sep 01 '11 at 09:09

score 2 · Answer 2 · answered Sep 01 '11 at 08:28

First of all, your code is vulnerable to SQL injection. Use PDO and prepared statements to fix this. Second of all, you're appearantly storing usernames unencrypted. This is very unsafe. Use a hashing function to encrypt the passwords, and encrypt the submitted password before running the query to get a match. Coloring the output is simple:

echo '<span style="color:red">Your not in</span>';

And use sessions to actually log the user in. After successfully querying the user table for the username/password combination, store the returned user_id in the $_SESSION variable. On each page that needs to be secured, just check for the existence of $_SESSION['user_id']; if it isn't there, your user needs to login so redirect him to the login form.

That should about do the trick for ya ;)

Using a simple user_id as a session key is a clear fail, it's a predictable integer, usually running in a sequence 1,2,3. You need to create a unique and random session_id. I would suggest using: `$session = hash('sha512', 'username'.$currentdate.$currenttime);` This will generate a unique session_id for every session, that connect be guessed. — Johan, Sep 01 '11 at 09:00
Well, I wasn't suggesting using the user_id as a session id... I said store the user_id in the session. The session_id generated by PHP is fine. — Rijk, Sep 01 '11 at 09:11

username and password validation in php mysql

2 Answers2