CODEWITHSUNDEEP
phppdo
Jason Chen
Jason Chen

Reputation: 2577

PDO if condition on row

I am in the process of making a login system using PDO. What I am finding is that users are able to log in despite their info not being in the database.

My PHP code is as follows:

 $error = "";
if(isset($_POST["submit"])){

    if(empty($_POST["youremaildesktop"]) || empty($_POST["yourpassworddesktop"]))
        {$error = "Both fields are required.";}

    else{

        $username=$_POST['youremaildesktop'];
        $password=$_POST['yourpassworddesktop'];

        $username = stripslashes($username);
        $password = stripslashes($password);
        $password = md5($password);

        /*PDO Version*/
        $resulter = $dbh->prepare("
            SELECT uid 
            FROM customer 
            WHERE username = '$username' 
            AND password = '$password'
        ");

 $resulter->fetchColumn();

if($resulter)
{
    $_SESSION['localstorage'] = $username;
    header("Location: userpage.php");
}
else { 
$error = "Incorrect username or password."; 
}

    }
 }

The $dbh PHP variable corresponds to my database connection credentials. The POST values correspond to my login form on my main page.

The goal is to connect users to userpage.php only if their credentials are correct. However, I am able to log in regardless of whether the user details correspond to users in my database.

Upvotes: 0

Views: 311

Answers (2)

Option
Option

Reputation: 2645

Here's how I would of tackled the situation myself..

if (isset($_POST['submit']))
{
    $username = $_POST['youremaildesktop'];
    $password = $_POST['yourpasswordesktop'];

    if (!empty($username) && !empty($password))
    {
        $res = $dbh->prepare("SELECT * FROM `customer` WHERE `username` = ?");
        $res->execute([$username]);

        $row = $res->fetch(PDO::FETCH_ASSOC);

        if ($res->rowCount() > 0)
        {
            if (password_verify($password, $row['password']))
            {
                $_SESSION['user_session'] = $row['uid'];

                header('Location: vm913.php');
            } else {
                // echo incorrect pass
            }
        } else {
            // echo no such user...
        }
    } else {
        // echo something...
    }
}

Edited as I missed an echo on the wrong password bit!

You'll see password_verify() rather than using md5 which really isn't recommended. You'll also see within my query I have bound the param.

Note: Obviously you will need to use password_hash() within the registration side of things before password_verify() will work.

Up to you at the end of the day, but it's always best practice to cover your backside.

I'd also, always assign the session to the userid rather than the username.

As an optional extra, may be worth adding login logs and failed logs to your system also.

Upvotes: 2

Jason Chen
Jason Chen

Reputation: 2577

I was able to fix this query by modifying my code to add the following:

$resulter->execute();           
$resulted = $resulter->fetchAll();

    if($resulted)
    {
        $_SESSION['localstorage'] = $username;
        header('Location:vm913.php');

    }
    else { $error = "Incorrect username or password."; }

Thanks to @Options as well for pointing out vulnerabilities in the code. I half posted this question also to see what people thought of it in terms of safety.

Upvotes: 0

Related Questions