PHP help needed for Login script

Question

When I login with the correct username and password I get an error "Wrong Username or Password" the database connection works and I think there is something wrong with the password + username check.

    <?php
    $host="mysql12-int.cp.hostnet.nl"; // Host name
    $username="u33936_mick"; // username
    $password="//password was correct"; // password
    $db_name="db33936_axe"; // Database name
    $tbl_name="users"; // Table name


    mysql_connect("$host", "$username", "$password");
    mysql_select_db("$db_name");



    $myusername = stripslashes($myusername);
    $mypassword = stripslashes($mypassword);
    $myusername = mysql_real_escape_string($myusername);
    $mypassword = mysql_real_escape_string($mypassword);
    $sql = 'SELECT * FROM `users` LIMIT 0, 30 WHERE username="$myusername" and        
    password="$mypassword"';
    $result=mysql_query($sql);


    $count=mysql_num_rows($result);



    if($count==1){
      session_register("username");
      session_register("password");
      header("location:index.php");
    } else {
      echo "Wrong Username or Password";
    }
    ?>

This is my form

     <form name="login" method="post" action="login.php">
                <fieldset id="inputs">
                    <input id="myusername" type="text" name="myusername"    
    placeholder="Username" required="">  
                    <input id="mypassword" type="password" name="mypassword"      
    placeholder="Password" required="">
                </fieldset>
                <fieldset id="login.php">
                    <input type="submit" id="submit" value="Login">
    </style>
                </fieldset>
            </form>

There is no more support for mysql_* functions, they are officially deprecated, no longer maintained and will be removed in the future. You should update your code with PDO or MySQLi to ensure the functionality of your project in the future. — War10ck
– War10ck, Commented Jun 27, 2013 at 20:45
@SenadMeškin this advice will be way more helpful (in the long run) than the solution. — Nir Alfasi
– Nir Alfasi, Commented Jun 27, 2013 at 20:48
doesn't the limit clause need to go at the end of the query? — user428517
– user428517, Commented Jun 27, 2013 at 20:48
@SenadMeškin Use of the deprecated mysql_ functions is a critical problem in any code. No solution is complete or even ethical when it does not entail advise on discarding deprecated techniques. The widespread use of mysql_ is dangerous to the language and to internet security in general, and is actually a bigger problem than whatever minor and temporary problem he is asking about here. He'll figure out the string concatenation with the answers he has, but if he doesn't figure out PDO he's got bigger problems. — Chris Baker
– Chris Baker, Commented Jun 27, 2013 at 20:57

Chris Baker · Accepted Answer · 2013-06-28 04:39:32Z

7

Foremost, I suggest you look in to transitioning away from the deprecated mysql_* family of functions in favor of mysqli (docs) or PDO (docs), neither of which require any significant change on your part as far as code goes.

As for your specific bug, it appears that you are incorrectly concatenating the values into the query. Also, your WHERE and LIMIT order is incorrect and invalid SQL. Here is the correct form:

$sql = '
    SELECT 
        * 
    FROM 
        `users` 
    WHERE 
        username="'.$myusername.'" AND 
        password="'.$mypassword.'"
    LIMIT 0, 30 
';

It is not clear what advantage you bring by using the LIMIT statement. Either you should have one matching row, or none. If anything, I would use LIMIT 1. If you got 30 rows back, what would you do with them!?

Converting to PDO is easy! The same query in PDO would look like this:

$host="mysql12-int.cp.hostnet.nl"; // Host name
$username="u33936_mick"; // username
$password="//password was correct"; // password
$db_name="db33936_axe"; // Database name

$pdo = new PDO('mysql:host='.$host.';dbname='.$db_name, $username, $password);
$sth = $pdo->prepare('
        SELECT 
            * 
        FROM 
            `users` 
        WHERE 
            username=:username AND 
            password=:password
        LIMIT 0, 30 
');
$sth->execute(array('username'=>$myusername, 'password'=>$mypassword));
$user = $sth->fetch();

Note that when you use PDO with bound parameters (shown here), you DO NOT have to sanitize with mysql_real_escape_string or addSlashes as you have done in your code.

It is not clear where you are defining $mypassword and $myusername, but if you are using registered globals then you should alter your code. Get the values directly from $_POST. addSlashes is NOT safe, and neither are registered globals.

Documentation

Deprecation notice for mysql_* functions - http://www.php.net/manual/en/function.mysql-query.php
PHP String operators - http://php.net/manual/en/language.operators.string.php
PHP PDO - http://php.net/manual/en/book.pdo.php
PHP mysqli - http://php.net/manual/en/book.mysqli.php

edited Jun 28, 2013 at 4:39

answered Jun 27, 2013 at 20:48

Chris Baker

50.8k13 gold badges99 silver badges116 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

user2528939 Over a year ago

thank you! but now my homepage displays that u33936_mick is logged in instead of the account i logged in with

Chris Baker Over a year ago

I think you're mixing up your database username & password with $_POST['myusername'] and $_POST['mypassword']. That is why using registered globals is a bad idea :) I don't know what's in your database, or what you do after the database check, but make sure you don't use $username anywhere, because that's the DB username, not the logged in user.

jeroen · Accepted Answer · 2013-06-27 20:50:20Z

2

A few problems:

You should use $_POST['myusername'] etc. to get the posted variables. If you are relying on register_globals you should turn that off as it is deprecated and poses a security risk;
The LIMIT clause comes at the end in mysql;
You should not modify the sent-in information using functions like stripslashes and escape functions, instead you should use prepared statements with bound variables in PDO / mysqli as the mysql_* functions are deprecated and passwords can contain for example slashes.

As a side-note, you should really salt and hash your password, don't store plain text passwords in your database.

answered Jun 27, 2013 at 20:50

jeroen

91.8k22 gold badges117 silver badges132 bronze badges

1 Comment

Chris Baker Over a year ago

Good note about registered globals! I added that to my answer as well, to be certain future readers get the hint. +1

Collectives™ on Stack Overflow

PHP help needed for Login script

2 Answers 2

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related