PHP Scripts Logs in even when the Login is incorrect

Question

I made a login script which works perfectly except the fact that it logs in even when the username and Password is incorrect.

Here is the code:

<?php
//SQL ENTRY
$username_db = "root";
$password_db = "";
$host = "127.0.0.1";
$db = "teach_login";

//Requested
$usern = $_POST['username'];
$pw = $_POST['password'];

//Make it safe
$usern = htmlspecialchars($usern);
$pw = htmlspecialchars($pw);
$pwmd5 = md5($pw);

//SQL SETTINGS  
$db_handle = mysql_connect($host, $username_db, $password_db);
$db_open = mysql_select_db($db, $db_handle);
echo $db_open."<br />";
if ($db_open){
    $SQL = "SELECT `username` FROM userpassword WHERE (username = '$usern' && password = '$pwmd5') ";
    $result = mysql_query($SQL);
    echo $result."<br />";;
    if ($result >= 1){
        $SQL_name = "SELECT * FROM `userpassword` WHERE (username = '$usern') ";
        $result_new = mysql_query($SQL_name);
        while($row = mysql_fetch_assoc($result_new)){
            $name = $row['full_name'];
            echo $name;
            echo "<br />";
            echo $row['password']."<br>";
            $SQL = "UPDATE `userpassword` SET `logged_in`=[1] WHERE `username` = '$usern' ";
            $result = mysql_query($SQL);
            if ($result > 0){
                mysql_close($db_handle);
            }else{
                echo "Data Not written";
            }
        }
        /*echo $result_new."<br />";            
        echo $result_name_array."<br />";
        $name = $result_name_array[1];
        echo $name."<br />";
        session_start();
        $_SESSION['login_name'] = $name;
        $_SESSION['login'] = 1;
        mysql_close($db_handle);
        //header ("location: teach_home.php");
        */
    }else{

        echo "Cannot Login";
        //header ("location: teach_login.php");
        mysql_close($db_handle);
    }

}else {
    echo ('DATABASE NOT FOUND');
    mysql_close($db_handle);
}
?>

The output is this which is the SQL ENTRY:

1<br>
Resource id #4<br>
Salik Sadruddin<br>
14918756cc99b9e6ce69f4c943680efc<br>
Data Not written<br>

There's quite a lot wrong with that script and the approaches you're taking, particularly from the point of view of good security. I'd recommend you use a pre-written library to handle authentication - there are some [good ones listed here](http://stackoverflow.com/questions/414034/actively-maintained-php-libraries-for-user-authentication) — Ian Gregory, Jun 10 '12 at 11:16
***For update, if UPDATE statement is succeeded $result will give you 0. For Insert it will give you 1*** — Fahim Parkar, Jun 10 '12 at 11:21
thank you but can u please tell me wher im wrong, for future use. — echo_salik, Jun 10 '12 at 11:22
Please stop writing new code with the ancient `mysql_*` functions. They are no longer maintained and community has begun the [deprecation process](http://goo.gl/KJveJ) . Instead you should learn about [prepared statements](http://goo.gl/vn8zQ) and use either [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli). If you cannot decide, [this article](http://goo.gl/3gqF9) will help to choose. If you care to learn, [here is a quite good PDO-related tutorial](http://goo.gl/vFWnC). — tereško, Jun 10 '12 at 11:23
If you're new to this then you really should switch to PDO_MYSQL or, alternatively, MySQLi. Using the MySQL extension is discouraged. See also [the PHP manual](http://www.php.net/manual/en/function.mysql-connect.php). — Arjan, Jun 10 '12 at 11:31
Please don't include signatures in your posts. The information in your user profile is sufficient and those who want to can read it there. Thanks. — Shadow The GPT Wizard, Jun 10 '12 at 11:35
@ZackValentine : see my answer… Let me know if you are not getting Data Updated as output. — Fahim Parkar, Jun 10 '12 at 11:39
You should consider not using plain MD5 to store your passwords. Because there are already huge lookup tables out there to revert commonly known passwords like `AdminSite`. — Gumbo, Jun 10 '12 at 11:43
@gumbo Not to mention the fact that md5 has been proven broken for some years now. And the fact that it can be bruteforced in a matter of seconds. — PeeHaa, Jun 10 '12 at 11:53
@RepWhoringPeeHaa It could be brute-forced all along. But meanwhile there are huge lookup tables and rainbow tables that *can* make brute-forcing obsolete. — Gumbo, Jun 10 '12 at 11:57
@Gumbo most people don't bother downloading rainbow tables, but just bruteforce it because it is faster. Not saying you are wrong though. — PeeHaa, Jun 10 '12 at 12:01
How to thank you all... i have now words other than THANK YOU!!! :D — echo_salik, Jun 10 '12 at 12:21
one more thing... mysqli works same as mysql codes right?? not going in the detain bt the syntax and stuff.. — echo_salik, Jun 10 '12 at 12:22

score 1 · Answer 1 · answered Jun 10 '12 at 11:24

This is where the flaw is:

$result = mysql_query($SQL);
if ($result >= 1){
    // …
}

The returned value of mysql_query is not the number of selected rows but:

For SELECT, SHOW, DESCRIBE, EXPLAIN and other statements returning resultset, mysql_query() returns a resource on success, or FALSE on error.

In your case the query will probably succeed but select no record, however mysql_query will return a resource that will fulfill the expression $result >= 1.

To fix this, use mysql_num_rows to get the number of selected rows:

if ($result && mysql_num_rows($result) === 1){
    // …
}

Also consider using MySQLi or PDO_MYSQL instead of standard MySQL extension. An you should also read about SQL injections as your current code is vulnerable.

Fahim Parkar · Answer 2 · 2012-06-10T11:29:56.393

0

For update, if UPDATE statement is succeeded $result will give you 0. For Insert it will give you 1

$SQL = "UPDATE `userpassword` SET `logged_in`=[1] WHERE `username` = '$usern' ";
        $result = mysql_query($SQL);
        if ($result == 0){
            echo "Data Updated";
            mysql_close($db_handle);
        }else{
            echo "Data Not written";
        }

edited Jun 10 '12 at 11:29

answered Jun 10 '12 at 11:24

Fahim Parkar

30,974
45
160
276

PHP Scripts Logs in even when the Login is incorrect

2 Answers2