Validating users with MySQL

Question

One of my friends told me that my validate.php file has some problems with SQL injections. Please feel free to commit it on GitHub.

 <?php
 session_start();
 error_reporting(0);
 include("../config.php");
 $conn = new mysqli($hostname, $username, $password, $db_name);
 $query = mysqli_query($conn,"SELECT * FROM `Administrators` WHERE username = '" . $_POST["usernamep"] . "' AND password='". md5($_POST["passwordp"]) . "'");
if(mysqli_num_rows($query) > 0){
$_SESSION["username"] = $_POST["usernamep"];
header("Location: dashboard.php");
}else{
header("Location: index.php?alert");
}
?>
?>

Can you please try and improve my code to prevent SQL injections?

• Phrancis: What happens if I enter Bobby'; DROP TABLE Administrators;-- as my user name? (hint: don't try it)
• Thomas Wilbur: Great idea let me fix it
• user34073: The password can mess your SQL up as well. Check the answers on this SO post out: stackoverflow.com/questions/332365/…
• Thomas Wilbur: github.com/poKe-Websites/poKe-Multimedia-editor/commit/…
• tim: @Phrancis probably nothing, mysqli_query doesn't support multiple queries as far as I know. Still, an attacker can bypass the login check, read out database information, and depending on settings write to files (meaning that they gain code execution).

Answer 1

It's not just your validate.php file, it's a problem accross your code. See eg here.

• always use prepared statements when putting variables in queries. It is not ok to just put them into queries, and addslashes is really not the proper solution either.
• don't use md5. It's too fast, it's broken, and it hasn't been a proper approach to hashing since at least 15 years. Use bcrypt instead.
• always die after a redirect. Clients do not have to follow them, meaning that any code that comes after a redirect can be executed. This may or may not be a problem right now, but it's just good practice.

Answer 2

As suggested by tim, you should be using prepared statements whenever you need to use user supplied input as part of a query, but at the very least, you should wrap any user supplied input in a mysql_real_escape_string() function.

Passing any type of raw user input to a database is asking for trouble. I would also suggest making use of the PHP PDO driver for databases, rather than they MySQLi driver, as when you change your code to properly use prepared statements rather than directly passing user input, you'll find that the PDO way of parameter binding is much more expressive than the MySQLi way.

• Thomas Wilbur: Thx Can You help with that
• jlswint: This is an excellent resource that should help you get started with PDO and using prepared statements to execute your queries.