5 Common Mistakes PHP Developers Make when Writing SQL

Do not use the old mysql api

There are several ways to connect to a MySQL database in PHP. The most common ones are the MySQL API, the MySQLi API and the PDO API (PHP Data Objects). The last two support more features than the old mysql API and are more secure. If you're using the old "mysql_" functions, you should stop and learn the new PDO API. Those old mysql functions are deprecated and are no longer supported in PHP 7.x.

Bad practice:

<?php  
$con = mysql_connect("localhost", "root", "mypass") or  
    die("Could not connect: " . mysql_error());  
mysql_select_db("tutorials");  
$result = mysql_query("select * from tutorials");  
echo "<h2>Here is a list of the topics:</h2>";  
while ($row = mysql_fetch_array($result)) {  
    echo $row['name']."<br />";  
}  
mysql_close($con);  
?>  

Better practice:

require_once('includes/conn.inc.php'); 
$sql= "SELECT name, age FROM employees WHERE company_id = 10"; 
$stmt = $pdo->query($sql); 
$row = $stmt->fetch(PDO::FETCH_ASSOC);
echo $row['name'];
echo $row[age];

Do not escape client input using escaping functions

Manually escaping query variables using mysql_real_escape_string is considered unsafe from two reasons:

1. When you use this method regularly, you are bound to miss it once. All an attacker needs is one loophole he can inject his code through to your SQL queries.
2. When using string variables, you should remember to use quotes, which is not very natural thing to do.

Instead, use prepared statements (see more information below).

Do not assume prepared statements are always safe in PHP

Prepared statements goal is to separate the query from the data, so that the data will be properly inserted to the query's parameters, without any manipulation options. In most cases, prepared statements are considered very safe to use and it's considered the best practice technique for injecting user-input parameters to queries.

Code sample:

require_once('includes/conn.inc.php'); 
$sql= "SELECT * FROM employees";
$stmt = $pdo->prepare($sql);
$stmt->execute();
$result = $stmt->fetchAll();
foreach($result as $row){
 echo "
<li>{$row['employeeName']}</li>";
}

So where prepared statements fall short in PHP? This is the case when you're injecting user inputs to a query, but prepared statements do not support that injection. For example, MySQL PDO doesn't support injecting a parameter (using the placeholder "?") in the LIMIT specifier. Also, user input cannot be inserted as table names or column names in the query. If you are using prepared statements in these cases, you should carefully sanitize the data manually, or better than that, use a library that does that for you and was already tested.

Do not assume ORM frameworks are not prone to SQL injection attacks

Using an ORM framework can be great (though debatable I'm sure). Said that, it doesn't mean that it's not prone to SQL injection attacks. It's true that it's harder to inject code into an ORM generated query, but it doesn't mean that a programmer can't make a mistake that will open a loophole. In most cases, this attack can be executed when concatenating user input to an ORM query, without using prepared statements. Yes, ORM frameworks such as Doctrine provide the ability to use prepared statements, so use it. Never concatenate strings to a query, whether it's an ORM query or raw SQL query.

By the way, it's not enough to follow the rules - make sure you test your ORM usage and implementation after coding, to make sure it's not open to any SQL injections.

Bad practice

In the following code example, you can see that a parameter that was inputed from the user is concatenated to the Doctrine DQL, which exposes the application to SQL injection.

<?php

// INSECURE
$dql = "SELECT u
          FROM MyProject\Entity\User u
         WHERE u.status = '" .  $_GET['status'] . "'
     ORDER BY " . $_GET['orderField'] . " ASC";

Bad practice

As recommended for non ORM users, using prepared statements is recommended when using an ORM framework (like Doctrine).

<?php

$orderFieldWhitelist = array('email', 'username');
$orderField = "email";

if (in_array($_GET['orderField'], $orderFieldWhitelist)) {
    $orderField = $_GET['orderField'];
}

$dql = "SELECT u
          FROM MyProject\Entity\User u
         WHERE u.status = ?1
     ORDER BY u." . $orderField . " ASC";

$query = $entityManager->createQuery($dql);
$query->setParameter(1, $_GET['status']);

Source of these examples: Doctrine documentation.

Do not underestimate the power of character encoding

According to OWASP (section 36.4.5, Use UTF-8 unless necessary), not using utf8mb4 will open you to various types of attacks (you can read more about encoding bypassing). Also, you should use utf8mb4 regularly in both PHP and MySQL for better and more standard multilingual support.

Code sample

$dsn = 'mysql:host=example.com;dbname=testdb;port=3306;charset=utf8mb4';

Summary

We looked at 5 very common mistakes many PHP developers do. Some of them happen because lack of knowledge (new frameworks are available all the time, make sure you know about them). Some happen because lack of experience (read, read and read more to make sure you use the safest and best APIs out there). Good luck in your next project!