login script redirects user no matter what

Question

I'm making a login page for my website which connects to a mysql database that stores the login details. However when I submit the form it redirects the information regardless of whether or not the login is valid. I'm very confused as to what could be wrong.

My form:

<form name="login" action="edit_profile/index.php" method="post"> //redirects to this page regardless of what has been submitted
    <fieldset>
        <legend>Please enter your login details</legend>
        <label for="username">Username</label>
        <input id="username" name="username" type="text" value="" required/><br>
         <label for="password">Password</label>
        <input id="password" name="password" type="password" value="" required/><br>
        <input type="submit" value="submit"/>
    </fieldset>

The pHp script:

<?php require_once "connectdb.php";?>
<?php
    if(isset($_POST["submit"])){
        $username=$_POST["username"];
        $password=$_POST["password"];

        $query=mysql_query("SELECT * FROM login_details WHERE Username='".$username."'AND Password='".$password."'");
        $numrows=mysql_num_rows($query);
    if($numrows!=0) {

        while($$row=mysql_fetch_asoc($query)) {
            $dbusername=$row["Username"];
            $dbpassword=$row["Password"];
        }

        if($user == $dbusername && $pass == $dbpassword) {
            session_start();
            $_SESSION["user_session"]=$user;
            echo "<pre>";
            print_r($_SESSION);
            echo '</pre>';
            }
        } else {
            echo "Invalid username or password!"; //does not echo
        }
}
?>

You are missing the name on your submit button. add name="submit" to your submit button. — Epodax, Apr 22 '15 at 13:24
you are calling $$row in your while statement - that's one $ too many — MuppetGrinder, Apr 22 '15 at 13:27
on a side note, you shouldn't SELECT * from a login table, you are pulling out the password if you do that. Massive security hole. — MuppetGrinder, Apr 22 '15 at 13:28
you should also be aware that you've left your code open to sql injection as you are not properly sanitizing user input. The 2nd check you have on checking the username and password is unnecessary as you've already validated they are a proper user via the database call, however it should be noted that you query may not be case sensitive due to the default collation being case insensitive. — Jonathan, Apr 22 '15 at 13:29

score 6 · Accepted Answer · edited May 23 '17 at 10:29

There are a few things wrong with your code.

One of which is with your conditional statement

if(isset($_POST["submit"])){...}

your submit button does not bear a name attribute

<input type="submit" value="submit"/>

modify it to read as

<input type="submit" name="submit" value="submit"/>

Then this while($$row remove one of the $

and mysql_fetch_asoc has a typo, it should read as mysql_fetch_assoc

while($row=mysql_fetch_assoc($query))

Plus, your posted code seems to be missing a closing </form> tag.

You should also check your action and make sure that is indeed the right URL for your PHP.
You should space out the AND in Username='".$username."'AND that may cause an issue Username='".$username."' AND, just in case.

Then we have this line:

if($user == $dbusername && $pass == $dbpassword)

$user and $pass at this point are undefined, so you may have wanted to use $username and $password respectively.

Add error reporting to the top of your file(s) which will help find errors.

<?php 
error_reporting(E_ALL);
ini_set('display_errors', 1);

// rest of your code

Sidenote: Error reporting should only be done in staging, and never production.

Also add or die(mysql_error()) to mysql_query().

Footnotes:

I noticed you may be storing passwords in plain text. If this is the case, it is highly discouraged.

I recommend you use CRYPT_BLOWFISH or PHP 5.5's password_hash() function. For PHP < 5.5 use the password_hash() compatibility pack.

Your present code is open to SQL injection. Use mysqli with prepared statements, or PDO with prepared statements. For password storage, use CRYPT_BLOWFISH or PHP 5.5's password_hash() function. For PHP < 5.5 use the password_hash() compatibility pack.

ircmaxell posted an answer yesterday using password_hash() and password_verify()
Have a look: https://stackoverflow.com/a/29778421/

score 0 · Answer 2 · answered Apr 22 '15 at 13:29

change this:

<?php
    if(isset($_POST["submit"])){

to this:

<?php
    if(isset($_POST["password"])&&isset($_POST["username"])){

to be sure that an empty form has not been posted.

Check the typo here:

while($$row=mysql_fetch_asoc($query)) {

changing it to

while($row=mysql_fetch_asoc($query)) {

login script redirects user no matter what

2 Answers2