Is there any SQL injection for this PHP login example?

Question

I want to write a login form, and I got one example from the web. I want to know, if there is any SQL injection for this code? If there is, what could the exploit's web form entry look like?

This is my form:

<form method="post" action="">
    <dt class="title"><label for="username">name:</label></dt>
    <dt><input type="text" name="username" id="username"  size="50"></dt>
    <dt class="title"><label for="password">pass:</label></dt>
    <dt><input type="password" name="password" id="password" size="50"></dt>
    <input type="hidden" name="post" value="1" />
    <input type="submit" name="submit" value="submit" class="button">
</form>

and this is my check:

if (($post[username]) AND ($post[password])) 
    {
        $query = 'SELECT * FROM `config` WHERE `config_admin_username`="'.$post[username].'" AND `config_admin_password`=MD5("'.$post[password].'") LIMIT 1';
        $sql_user_check = $db->fetch($query);
        if (!$sql_user_check) { 
            $error .= "Wrong Entry.<br />";
        } else {
            $_SESSION[admin] = 1;
            header("Location:index.php");
            exit;
        }
    }

I'm using phpmyadmin 3.2.0.1 on wamp 2.0i

Answer 1

You've come to the right place. Welcome to IT security!

is there any sql injection for this code?

Yes

if there is what is that Entry?

username: [any username from your website]" /*
password: sux0r")*/ OR ("1"="1

It will run this query:

SELECT * FROM `config` 
WHERE `config_admin_username`="[any username from your website]" /* 
      AND `config_admin_password`=MD5("sux0r")*/ OR ("1"="1") LIMIT 1'

If we remove the commented-out parts that SQL engine won't parse, this results in:

SELECT * FROM `config` 
WHERE `config_admin_username`="[any username from your website]" OR ("1"="1") LIMIT 1'

SQL query will be parsed up till the commented out part (I used the /* and */ start-comment/end-comment pair in my example and assuming you're using MySQL, but that can be different depending on RDBMS used), effectively disabling your password check altogether. Even if in-SQL commenting is not supported, or disabled, the options to exploit your login are really limitless. I won't even go into your choice of MD5 as a password hashing algorithm, as you have a lot of other things to consider first. It clearly isn't the recommended hash algorithm for password hashing though.

You've done good to have come to this website and that you're asking yourself questions regarding your code safety. Congratulations, you're already a better PHP programmer than most that never got so far are! Now, it's up to you what you'll learn from already existing questions and answers here. Some tags to consider are php, authentication, web-application, hash,...

Answer 2

TildalWave has a great answer, but there is a great mediation to SQL injection that was missing from the answer. Prepared Statements. As Tildal said the entire statement with user input is parsed with no parsable distinction between variables and constant parts of the statement. The solution is sending the constant part of the statement with variable placeholders then the variables. PDO is what I use for my SQL interaction. Here is the function I use...

function _SQL_QUERY($_base_request, $_request_arguments)
{
                $_DB_USERNAME = "bob"; $_DB_PASSWORD="secret"; $_DB_LOCATION="localhost"; $_DB_NAME="MYWICKEDDATABASE"; $_DB_SERVER_TYPE="mysql";
                if(!isset($_DATABASE_CONNECTION))
                {
                        $_DATABASE_CONNECTION = new PDO("$_DB_SERVER_TYPE:dbname=$_DB_NAME;host=$_DB_LOCATION;charset=utf8", $_DB_USERNAME, $_DB_PASSWORD);
                        $_DATABASE_CONNECTION->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
                }
                $__STMT = $_DATABASE_CONNECTION->prepare($_base_request);
                if($__STMT->execute($_request_arguments))
                {
                        return $__STMT;
                }
                else
                {
/*
        //Helpful ONLY when debugging!!!
                        echo 'ERROR</ br>';
                        echo $_base_request;
                        echo '</ br>';
                        print_r($__STMT->errorInfo());
                        print_r($_request_arguments);
                        die('</ br>');
*/
                        return false;
                }
}

to use it you would just need to run this

_SQL_QUERY("Select * FROM names WHERE name=:var1 AND age=:var2", array('var1'=>'jim','var2'=>50));

There are plenty of improvements to be made (mainly functionaly) to this but it is a vast improvement to the traditional SQL functions. BTW its important to know that if a row has a failed attempt at an injection in it's data, a backup and restore may actually make it successful depending on the method used. Kudos for doing a security audit of your code, if only more people learned programming with that motivation towards security.

Answer 3

Yes it is. You are not validating anything. You have to use

• mysql_real_escape_string to prevent SQL injection
• htmlentities/htmlspecialchars to prevent XSS attacks

Take a look at this tutorial as it helps on learning to prevent SQL injection.