Does this generic query follow good OOP standards?

Question

I am trying to convert over to MySQLi and wanted expert advice. Is the enclosed code 100% OOP. Also, how secure is the code against attacks? I know nothing is 100% secure, but how good/safe is it? How and what can I make better? This is just a generic query as I wanted to get a basic model setup.

<?php
define ('DB_HOST', 'localhost');                       //  Host name
define ('DB_USER', 'root');                            //  Mysql username
define ('DB_PASSWORD', 'password');                    //  Mysql password
define ('DB_NAME', 'database');                        //  Database name

// New Connection
$mysqli = new mysqli(DB_HOST,DB_USER,DB_PASSWORD,DB_NAME);

require("dbcon.php");  // This is actually the code above in a separate file that I have included here for checking.

$query = "SELECT id, username, firstname FROM users WHERE firstname=?";

if ($stmt = $mysqli->prepare($query))
{
   // Bind the variables
   $firstname = $_POST[first_name];

   // Bind parameters  s - String, b - Boolean, i - Integer etc
   $stmt->bind_param("s", $firstname);
   // Execute it
   $stmt->execute();
   // Store it for num rows
   $stmt->store_result();
   $num_rows = $stmt->num_rows;
   // Assign blank variables
   $id = "";
   $username = "";
   $firstname = "";
   // Bind the result and retrieve the data
   $stmt->bind_result($id, $username, $firstname);
      if(($num_rows) > 0)
      {
         while($stmt->fetch())
         {
            echo $id;
            echo "<br />";
            echo $username;
            echo "<br />";
            echo $password;
            echo "<br />";
            echo "Number of rows: ", $num_rows;
            echo "<br />";
            echo "<br />";
         }
      }
      else
      {
         echo "No records";
      }
   // Close the statement
   $stmt->close();
}
// Close connection
$mysqli->close();
?>

Answer 1

Your binding of parameters will help in preventing SQL injections.

You need to sanitize your data first though. You need to treat everything that comes from a user (passed from the client machine) as 'dirty' and not allow it into your system before it's been 'cleaned'. Your first step would be to clean it - converting all HTML tags and special characters into their HTML entities htmlspecialchars() htmlentities() (well as long as HTML isn't allowed). This will help prevent cross-sight scripting (XSS). Typically then you would move on to validating the data - making sure it's in a format you expect. Then if it's not you return it to the user with an error. This is also assuming that you've checked the user has permission to run this function, if relevant.

Answer 2

Yes, it looks like you're exclusively using OO style.

Your code will prevent SQL injection attacks by using MySQLi prepared statements as you do, but you're not doing anything to prevent a potential XSS attack through the output you're sending to the browser. To do this, escape characters that could be used in an XSS attack before sending them to the browser, for example

echo htmlentities($username);

It's unclear whether this is code you intend to use or if you're just illustrating concepts, but what you've posted exposes usernames to anybody who can access the script in a browser. You need do several things if you're actually going to run this code. The first that comes to mind is checking to see if the user is logged in under an account allowed to view usernames.

Also I see a variable named $password although it's not part of the select. It's unclear why you have this variable, but you should never store passwords in a database without salting and hashing them - which makes it useless to display in the browser as you seem to want to do here. Here is an article on the topic: https://crackstation.net/hashing-security.htm