Simple login checking password

Question

What am I doing wrong here:

<?php
    if (isset($_POST['submitted'])) {

    $errors = array();
        require_once ('mysql_connect.php');

    session_start();
    $username = $_POST["username"]; // This is the inputted username from the form in Login.html
    $password = $_POST["password"]; // This is the inputted password from the form in Login.html


    if (empty($errors)) {
        $query="SELECT username FROM users WHERE username='$username' AND password='SHA($password)'"; 

        $result = mysql_query($query);  

        // Mysql_num_row is counting table row

        if (mysql_num_rows($result) == 1) {
                $_SESSION["username"] = $username; // Creates a cookie saving the username
                $_SESSION["loggedIn"] = true; // Creates a cookie saying the user is logged in
            // Show thank you message
            echo '<h3 style="color:green;">Thank You!</h3>
            <span style="color:green;">You have been logged in.</span>';
        } else {
            echo '<font color="red">You could not be logged in, please make sure your username and password is correct.</font>';
            foreach ($errors as $msg) {
            echo " - <font color=\"red\">$msg</font><br />\n";
            }
        }

} else {
        echo '<font color="red"><h3>Error!</h3>
        The following error(s) occured:<br /></font>';

        foreach ($errors as $msg) {
            echo " - <font color=\"red\">$msg</font><br />\n";
        }
    }
}
?>

I get a:

Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in /login.php on line 19

Also is the way I SHA the password correct?

Your mysql query probably has an error. Remove the `@` in from the `mysql_query` and see if there's an error. — Nadh, Apr 28 '12 at 12:11
Perhaps `mysql_query` is failing. Remove the `@` to see the error message. — Oliver Charlesworth, Apr 28 '12 at 12:11
I have added more code, I dont get a mysql_num_rows warning anymore but the login still fails. — Anicho, Apr 28 '12 at 12:22

score 3 · Accepted Answer · edited May 23 '17 at 11:55

The problem is that your MySQL query is causing an error, which means that your $result doesn't actually contain a result resource.

You need to remove the '' from around SHA($password) in your query, and instead put them around the password value, like so:

$query="SELECT username FROM users WHERE username='$username' AND password=SHA('$password')";

Also is the way I SHA the password correct?

That depends on how the passwords were hashed when they were inserted into the database. MySQL's SHA() is the same as its SHA-1():

Calculates an SHA-1 160-bit checksum for the string, as described in RFC 3174

Which is also the same as PHP's sha1(); so, for example, if the passwords in the database are SHA-1 hashes that were created using PHP's sha1(), it should be fine.

Side Notes

You should use PHP's crypt() or hash() for hashing passwords, rather than SHA-1. For example usage, refer to the PHP documentation.
You should be cleaning/escaping all user-provided data before using any of it in a database query. This is to stop SQL injection attacks. Better yet, use prepared statements and parameterized queries. Refer to this answer for more information.

`$query = "INSERT INTO users (username, email, password, isadmin) VALUES ('$username', '$email', SHA('$password'), 'false')";` my insert statement — Anicho, Apr 28 '12 at 12:23
@Anicho Using `SHA()` in your `SELECT` query should work then. — Xenon, Apr 28 '12 at 12:29
thanks for your help! its amazing how you can learn so much from one post on overflow! — Anicho, Apr 28 '12 at 12:44

score 1 · Answer 2 · answered Apr 28 '12 at 12:13

1

Don't use '' by SHA function

$query="SELECT username FROM users WHERE username='$username' AND password='SHA($password)'";

And offcourse don't remember escape your data.

answered Apr 28 '12 at 12:13

yAnTar

3,609
9
41
64

Yes, I hurried and made a mistake – yAnTar Apr 28 '12 at 13:17

score 1 · Answer 3 · answered Apr 28 '12 at 12:14

1

Why did you put a @ in @mysql_query($query); ? If you have a MYSQL error you should handle it correctly and not ignore it (I assume you have an error there). It would help you understand where your bug is coming from.

Also, you can do the SHA in PHP (which depends on your architecture could be better or worse for your project).

http://php.net/manual/en/function.sha1.php

answered Apr 28 '12 at 12:14

Yaniv Peer

141
5

I am .net developer but this weekend I thought I would dabble in some php its all a learning curve. I appreciate your tips! – Anicho Apr 28 '12 at 12:16

Simple login checking password

3 Answers3