sql injection/login page, query about my code

Question

Good morning all,

I recently made a post on this website regarding a login page for a website. It was pointed out to me that I was very vulnerable to an SQL injection. I have spent the past weekend researching into SQL injections and I am getting a bit of a better idea about how they function, however I am still very new to PHP (I taught myself the basics in a day). I was wondering if anyone could help me with my code please.

I have read every link that people posted and researched it till my head exploded (no need to edit, its not literal), but I am still struggling as to the code itself.

Here is my code:

<?php
    session_start();
    // dBase file
    include "dbConfig.php";

    if ($_GET["op"] == "login")
    {
       if (!$_POST["username"] || !$_POST["password"])
       {
         die("You need to provide a username and password.");
       }

       // Create query
       $q = "SELECT * FROM `dbusers` "
       ."WHERE `username`='".$_POST["username"]."' "
       ."AND `password`=PASSWORD('".$_POST["password"]."') "
       ."LIMIT 1";
       // Run query
       $r = mysql_query($q);

      if ( $obj = @mysql_fetch_object($r) )
      {
        // Login good, create session variables
        $_SESSION["valid_id"] = $obj->id;
        $_SESSION["valid_user"] = $_POST["username"];
        $_SESSION["valid_time"] = time();

        // Redirect to member page
        Header("Location: members.php");
      }
      else
      {
        // Login not successful
        die("Sorry, could not log you in. Wrong login information.");
      }
   }
   else
   {
    //If all went right the Web form appears and users can log in
    echo "<form action=\"?op=login\" method=\"POST\">";
    echo "Username: <input name=\"username\" size=\"15\"><br />";
    echo "Password: <input type=\"password\" name=\"password\" size=\"8\"><br />";
    echo "<input type=\"submit\" value=\"Login\">";
    echo "</form>";
   }
 ?>

Now I know it needs validation AND sanitisation in PDO, am just struggling as to what to actually write in my code. I am hoping that someone could help rather than just link me to another page please

as an edit, if anyone has a link to a tutorial about logins which are SQLinjection safe that could help me/other people looking to protect against that would be much appreciated. Thanks

Answer 1

Well you can use PDO or mysqli. The most interesting point is that you can use prepared statements with both.

If you used mysql before, as in your code, then mysqli is easier to understand for you.

The manual page of PHP has good examples for mysqli with prepared statements.

First of all you need a mysqli connection:

$mysqli = new mysqli("host", "user", "password", "database");

When you got your query, replace all external inputs with a ?.

$q = "SELECT * FROM `dbusers` "
       ."WHERE `username`=? "
       ."AND `password`=PASSWORD(?) "
       ."LIMIT 1";

And create a prepared statement:

$stmt = $mysqli->prepare($q);

Then you can easily bind your params. Because in your case both are strings, you have to use a s for each param.

$mysqli->bind_param('ss', $_POST['username'], $_POST['password']);

And then just execute the statement:

$stmt->execute();

With methods like fetch() you can get the result(s).

Answer 2

As you're using the old mysql_* API, you could just use the mysql_real_escape_string function which does exactly what you need.

Use it like this :

    $q = "SELECT * FROM `dbusers` "
   ."WHERE `username`='".mysql_real_escape_string($_POST["username"])."' "
   ."AND `password`=PASSWORD('".mysql_real_escape_string($_POST["password"])."') "
   ."LIMIT 1";

Answer 3

try this function to pass every POST variable

function mysql_string($str)
{
$str = stripslashes($str);
$str = mysql_real_escape_string($str);
return $str;
}

now use it as follows

$username = mysql_string($_POST["username"]);
$password = mysql_string($_POST["password"]);

$q = "SELECT * FROM `dbusers` "
       ."WHERE `username`='".$username."' "
       ."AND `password`=PASSWORD('".$password."') "
       ."LIMIT 1";

no use $username in your query to prevent mysql injection

Answer 4

Just as an example here is a PDO INSERT statement

$dbHost = "";
$dbName = "";
$dbUser = "";
$dbPass = "";

$dsn = 'mysql:host=' . $dbHost . ';dbname=' . $dbName;
$options = array(
PDO::ATTR_PERSISTENT => true,
PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
);

try {
$dbh = new PDO($dsn, $dbUser, $dbPass, $options);
$db = $dbh->prepare('INSERT INTO db_table (column_one, column_two, column_three)
VALUES (:column_one, :column_two, :column_three)');
$db->bindParam(':column_one', $dataForColumnOne);
$db->bindParam(':column_two', $dataForColumnTwo);
$db->bindParam(':column_three', $dataForColumnThree);
$db->execute();
} catch (PDOException $e) {
echo $e->getMessage();
}

An example here is a PDO multiple row fetch statement

try {
$dbh = new PDO($dsn, $dbUser, $dbPass, $options);
$db = $dbh->prepare('SELECT * FROM db_table');
$db->execute();
$rows = $db->fetchAll(PDO::FETCH_ASSOC);
foreach ($rows as $row) {
print_r($row);
}
} catch (PDOException $e) {
echo $e->getMessage();
}

As a rule never trust user input, Notice that the data is added via "->bindParam" and not directly inserted into the "->prepare" statement

Please refer to PHP PDO