Tuesday 28 June 2016

Spot The Bug: SQL weirdness

It's time for Spot The Bug again, so get your game face on.

I'm looking at you,
buster.

I was working on a very bare-bones login procedure. Quick, dirty, throw-away code. The front-end was in HTML, back-end in PHP and interfacing with a MySQL database. Upon clicking the Login button, the login.php script would fire off and query the database. The query returns the number of records that match the email address and password. If there is exactly one match, you're logged in. And if not, you have to try again.

Sounds simple enough. I must've done it a million times. But somehow, I just could not get a match. I always got the message "No such user found.". No syntax errors were in my PHP script.

But then I discovered something really strange with the query. This is the original code from login.php.
<?php
include "inc/config.php";

$login = $POST["txtLogin"];
$password = $POST["txtPassword"];

if (db_connect()->connect_errno)
{
    die("Database connection error.");
}
else
{
    $DBConn=db_connect();

    $strsql="SELECT COUNT(user_id) FROM tb_users WHERE user_login LIKE ? AND  user_password=MD5(?)";

    $sqlresult = $DBConn->prepare($strsql);

    if (!$sqlresult)
    {
        die("Database query error.");
    }
    else
    {
        $sqlresult->bind_param("ss",$login,$password);

        $sqlresult->execute();
        $sqlresult->bind_result($num);
        $sqlresult->fetch();

        if ($num==1)
        {
             //Logged in
        }
        else
        {
             die("No such user found.");
        }
    }

    $DBConn->close();
}
?>

When I changed it, like so, I got a match!
<?php
include "inc/config.php";

$login = $POST["txtLogin"];
$password = $POST["txtPassword"];

if (db_connect()->connect_errno)
{
    die("Database connection error.");
}
else
{
    $DBConn=db_connect();

    $strsql="SELECT COUNT(user_id) FROM tb_users WHERE user_login = ? AND  user_password=MD5(?)";

    $sqlresult = $DBConn->prepare($strsql);

    if (!$sqlresult)
    {
        die("Database query error.");
    }
    else
    {
        $sqlresult->bind_param("ss",$login,$password);

        $sqlresult->execute();
        $sqlresult->bind_result($num);
        $sqlresult->fetch();

        if ($num==1)
        {
              //Logged in
        }
        else
        {
              die("No such user found.");
        }
    }

    $DBConn->close();
}
?>

But it is not enough that things work; sometimes it is just as important to know why they are working. A SQL LIKE is supposed to be more flexible than the "=" operator. So why was the "=" operator matching while the LIKE wasn't?

What went wrong

The problem wasn't in my output. It was in my input. Apparently, while entering the email address in my HTML form, I had accidentally included a trailing space. Instead of "teochewthunder@gmail.com", the input was "teochewthunder@gmail.com ". And this had resulted in the SQL query returning a false positive. The Microsoft Knowledge Base provides a decent explanation. Apparently SQL Server suffers from the same quirk as MySQL. (https://support.microsoft.com/en-us/kb/316626).


How I fixed it

The problem arose because, unlike all the other times I had built this seemingly simple module, I had not written a utility to sanitize the input first. Which included eliminating leading and trailing spaces! So after I added this to my PHP script, it ran like a charm. It was a basic sanitization function using PHP's trim() function to remove leading and trailing spaces.
<?php
include "inc/config.php";

$login = sanitize($POST["txtLogin"]);
$password = sanitize($POST["txtPassword"]);

if (db_connect()->connect_errno)
{
    die("Database connection error.");
}
else
{
    $DBConn=db_connect();

    $strsql="SELECT COUNT(user_id) FROM tb_users WHERE user_login LIKE ? AND  user_password=MD5(?)";

    $sqlresult = $DBConn->prepare($strsql);

    if (!$sqlresult)
    {
        die("Database query error.");
    }
    else
    {
        $sqlresult->bind_param("ss",$login,$password);

        $sqlresult->execute();
        $sqlresult->bind_result($num);
        $sqlresult->fetch();

        if ($num==1)
        {
              //Logged in
        }
        else
        {
              die("No such user found.");
        }
    }

    $DBConn->close();
}

function sanitize($input)
{
    $temp=$input;
    $temp=trim($temp," ");

    return $temp;
}

?>


Moral of the story

Two things to take away from this.

1) Always, always sanitize user input. If I, the developer, can make such an elementary mistake on my own bloody form, who knows what the typical clueless end-user is capable of?

2) Beware of the SQL LIKE and "=" operator. They don't always function the way you'd expect.

That was, like, totally bogus, dudes.
T___T

No comments:

Post a Comment