Mysql Login Form

Question

So I made this basic login form as follows in html.I understand that it is lacking at the moment in security measures, but I just want it to simply be able to login first. Then I will build in the more advanced features.

<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
<title>Jotorres Login Form</title>
</head>
<body>
    <form method="post" action="login.php" >
        <table border="1" >
        <tr>
            <td><label for="users_name">Username</label></td>
            <td><input type="text" 
              name="users_name" id="users_name"></td>
        </tr>
        <tr>
            <td><label for="users_pass">Password</label></td>
            <td><input name="users_pass" 
              type="password" id="users_pass"></input></td>
        </tr>
        <tr>
            <td><input type="submit" value="Submit"/>
            <td><input type="reset" value="Reset"/>
        </tr>
    </table>
</form>
</body>
</html>

Then I call login.php. I have a user in the RDS database called "tommytest" with a password of "tommytest" but it isn't working. Keeps saying incorrect username or password. But if I leave the form blank and hit submit it says I am verified.

<?php

// Grab User submitted information
$name = $_POST["users_name"];
$pass = $_POST["users_pass"];

// Connect to the database
$con = mysql_connect("localhost","username","password");
// Make sure we connected succesfully
if(! $con)
{
    die('Connection Failed'.mysql_error());
}

// Select the database to use
mysql_select_db("ist421test",$con);

$result = mysql_query("SELECT userName AND password FROM users_tbl WHERE userName = '$name'");

$row = mysql_fetch_array($result);

if($row["userName"]==$name && $row["password"]==$pass)
    echo"You are a validated user.";
else
    echo"Sorry, your credentials are not valid, Please try again.";
?>

Well, if you leave the form blank, the query result would be blank. And so would $name and $pass. So when comparing an empty $name to an empty $row['userName'], that equation is TRUE. Blank does equal blank. I'll put a more accurate method below — Rottingham, Mar 18 '14 at 23:19
Use `WHERE userName = '$name' AND password='$pass'");` or `WHERE userName = '$name'");` it needs to be wrapped in quotes. — Funk Forty Niner, Mar 18 '14 at 23:20

Mike Brant · Answer 1 · 2014-03-19T00:34:00.497

So you have a series of issues here:

You are using mysql_* functions which are deprecated. You should consider using mysqli or PDO.
You are doing absolutely nothing to prevent SQL injection. You should do a quick search on SQL injection here in SO to see how to solve for this.
You are using a plaintext password, when you should be hashing the password. You should look at password_hash() and password_verify() functions in PHP documentation to see how to properly hash your passwords.
You have not enclosed the $name value in single quotes in your SQL. And you are not specifying select fields correctly (like SELECT userName, password ...). Also, you are not handling any errors resulting from your query or you would have seen this. This is actually the root cause for your code not working as expected. You should always check the result of any DB function call to make sure you are getting the expected result before doing any more with it.
You are doing nothing to check that a form value is actually entered. This, along with your bad query result noted above and the fact you are using loose comparisons, give you the validated user text when no values have been input. You really should not even make a query against the database if you have empty POST fields (for either required field).
You also have a cross-site request forgery (CSRF) vulnerability as well. So once you have figured out the basic login functionality, you may want to research on how to use session tokens to mitigate against that risk.

I don't mean to overwhelm you with a bunch of items that are not specific to answering your question. It is clear that you are in the learning mode here, so I just wanted to make sure you had some good guidance on some of the other issues your code currently has beyond the simple SQL syntax issues. Hopefully you can take this information to help you form a better picture of what sort of issues need to be considered when you start working with user logins in a web application.

score 2 · Answer 2 · answered Mar 18 '14 at 23:21

2

You are not keeping the password stored safe. Hash+salt needed
Look at your query, you are not checking the combination of username and password, only the username
You should read about SQL Injections
mysql_* is deprecated. Use PDO or mysqli

Now, what might be wrong? Try printing mysql_error() to debug.

SELECT username,password FROM users_tbl WHERE username='username' AND PASSWORD='password'

Now, if this query results in one row, the combination is correct: mysql_num_rows() will give you the number of rows.

Again, check out PDO or Mysqli.

Good luck.

answered Mar 18 '14 at 23:21

msfoster

2,474
1
17
19

Your information is largely correct. I will caution against advising the OP to have both password and username in WHERE clause, as if they are using a proper hashing algorithm with salt (i.e. like created with `password_hash()`), they would have no way to generate a matching password for the WHERE filter until they actually get the password/salt from the database. They have to verify the password as a second step after the row is returned. – Mike Brant Mar 18 '14 at 23:26
You're absolutely right. I just wanted to point out how the query he was trying to accomplish could look like. – msfoster Mar 18 '14 at 23:31
Would like to mention, using username and password in WHERE clause would work fine here, because the OP isn't salting or hashing. Just bad practice. – Nazca Mar 20 '14 at 16:29

score 1 · Accepted Answer · edited May 23 '17 at 12:20

Firstly, do consider using a safer password storage method such as crypt(), bcrypt() or PHP's password_hash() function (which has already been addressed).

This line is where the biggest problem is: (using AND instead of a comma for your column names.)

"SELECT userName AND password FROM users_tbl WHERE userName = '$name'"
                 ^^^

Then, you did not quote $name in WHERE userName = $name

Use:

"SELECT username, password FROM users_tbl WHERE username = '$name' AND password='$pass'"

Here is a mysqli_* based version, with the PHP/SQL/HTML form in one file for testing.

(Tested)

<?php
DEFINE ('DB_USER', 'xxx');
DEFINE ('DB_PASSWORD', 'xxx');  
DEFINE ('DB_HOST', 'xxx');
DEFINE ('DB_NAME', 'xxx');

$mysqli = @mysqli_connect (DB_HOST, DB_USER, DB_PASSWORD, DB_NAME) 
OR die("could not connect");

if(isset($_POST['submit'])){
// Grab User submitted information
$name = mysqli_real_escape_string($mysqli,$_POST["users_name"]);
$pass = mysqli_real_escape_string($mysqli,$_POST["users_pass"]);

$result = mysqli_query($mysqli,"SELECT username, password FROM users_tbl WHERE username = '$name' AND password='$pass'");

$row = mysqli_fetch_array($result);

if($row["username"]==$name && $row["password"]==$pass){
    echo"You are a validated user.";
    }
else{
    echo"Sorry, your credentials are not valid, Please try again.";
    }

} // if(isset($_POST['submit'])){
?>

<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
<title>Jotorres Login Form</title>
</head>
<body>
    <form method="post" action="" >
        <table border="1" >
        <tr>
            <td><label for="users_name">Username</label></td>
            <td><input type="text" 
              name="users_name" id="users_name"></td>
        </tr>
        <tr>
            <td><label for="users_pass">Password</label></td>
            <td><input name="users_pass" type="password" id="users_pass"></input></td>
        </tr>
        <tr>
            <td><input type="submit" name="submit" value="Submit"/>
            <td><input type="reset" value="Reset"/>
        </tr>
    </table>
</form>
</body>
</html>

Footnotes:

Consider using mysqli_* functions with prepared statements or PDO, mysql_* functions are deprecated and will be deleted from future PHP releases.

See also: crypt(), documentation on bcrypt() on SO, and PHP's password_hash() function.

Thank you. This worked very well for me. I know I need to update my methods, just a simple project I was working on. — h3tr1ck, Mar 19 '14 at 13:50

score 0 · Answer 4 · answered Mar 18 '14 at 23:25

Instead of the hoops you are jumping, I would do something similar to this, EVEN though this is prone to SQL injection, which I'll talk about afterwards.

SELECT COUNT(*) FROM `users_tbl` WHERE `userName` = '$name' AND `password` = '$pass'

So you would have this

$result = mysql_query("SELECT COUNT(*) 
    FROM `users_tbl` 
    WHERE `userName` = '$name' AND `password` = '$pass'");

if (!$result || mysql_num_rows($result) <= 0) {
    // NO user found!
} else {
    // User found and password matches
}

This is not entirely a safe way of working with SQL - because you are directly injecting the value that a user types in, right to the database query. A hackster could type DROP TABLE users_tbl as his username and potentially empty your database.

On top of that, you are using deprecated mysql_ methods that are being removed completely from PHP - hence the big red box when you look at any online documentation regarding those methods.

http://us2.php.net/manual/en/function.mysql-query.php

I hope this answer helps you understand the query problem, and a little error checking, but before progressing, you should invest some hours into catching up on the modern mysqli and PDO methods.

Please note that if the OP uses a properly hashed password (i.e. one with a random salt), they will not be able to perform a query using the password value in the WHERE clause. They must retrieve the password from the query and verify it. — Mike Brant, Mar 18 '14 at 23:28
@MikeBrant Uh Mike, I totally agree with you man - but that's far beyond the OP, don't ya think? He has some hurdles to hop before he gets to properly salted passwords and how they are stored. I would never even use a query like this in the most basic project, but was simply showing why and a way to see where is errors were at. — Rottingham, Mar 18 '14 at 23:29
Yes and no. It is clearly beyond their current understanding of how to work with login operations on a database. that being said, if you are going to offer suggestions, I feel you should offer suggestions that move the user in the right direction. Using both username and password in the query doesn't move them in the right direction. All your other points are very good. — Mike Brant, Mar 18 '14 at 23:33

score 0 · Answer 5 · answered Mar 18 '14 at 23:26

0

You're not retrieving "password" correctly on your query, so always is empty $row['password'] and just an empty value will validate. change your query like

SELECT userName , password FROM users_tbl WHERE userName = '$name'

answered Mar 18 '14 at 23:26

Olvathar

551
3
10

Mysql Login Form

5 Answers5