php - Switching to Prepared Statements

Question

I have just started working on my first project (for fun). I am learning PHP and MySQL and have sort of completed my first working application. It works, but I now am learning how to secure my application and thus prevent SQL injections. I have roughly 50+ PHP files that manage interaction with my MySQL database. They all look something like this:

<?php
$inputvalues = $_POST;
$errors = false;
$result = false;
session_start();
$uid = $_SESSION['usr_id'];
$mysqli = new mysqli('localhost', "root", "", "testdb");

if (mysqli_connect_errno()) {
        printf("Connect failed: %s
", mysqli_connect_error());
        exit();
    }

    foreach ($inputvalues as $key => $value) {
        if(isset($value) && !empty($value)) {
            $inputvalues[$key] = $mysqli->real_escape_string( $value );
        } else {
            $errors[$key] = 'The field '.$key.' is empty';
        }
    }

    if( !$errors ) {
        $addresult = "
            SELECT a.firstnames, a.surname, a.schoolrole, a.datejoined FROM teachers a LEFT JOIN schools b ON a.schoolid = b.id WHERE b.id = '".$inputvalues['schoolid']."'        
         ";

         if( $result = $mysqli->query($addresult) ) {
            while($row = $result->fetch_all())
            {
                $returnResult = $row;
            }
        }
    }
    mysqli_close($mysqli);
    echo json_encode(['result' => $returnResult, 'errors' => $errors]);
    exit;
?>

This is the format I have used throughout my application for reading and writing data to/from the database. If I need to change them to prepared statements, where I am not inserting any information but just retrieving them, how would I go about it?

Also, If I am not entering any data to the DB, is it still vulnerable to injection?

Could you kindly provide me with an example on how I can adapt my current code to prepared statements, I would really appreciate it.

Answer 1

I have been in the same situation. I was using concatenated statements too, then I switched my application to prepared statements.

the bad news is you are going to change every SQL statement built by concatenating client data to the SQL statement, which almost will be every SQL statement you have in your 50 source files.

the good news is the gain from switching to prepared statements is priceless, for example:

1-you will never be worried about something called "SQL Injection attack"

the php manual says

If an application exclusively uses prepared statements, the developer can be sure that no SQL injection will occur (however, if other portions of the query are being built up with unescaped input, SQL injection is still possible).

For me, that reason -peace of mind- is enough to pay the cost of changing my source code. , now your clients can type in a form name field robert; DROP table students; -- ;) and you feel safe that nothing is gonna happen

2- you don't need to escape the client parameters anymore. you can directly use them in the SQL statement, something like :

$query = "SELECT FROM user WHERE id = ?";
$vars[] = $_POST['id'];

instead of

$id = $mysqli->real_escape_string($_POST['id']);
$query = "SELECT FROM user WHERE id = $id";

which is something you had to do before using prepared statements, which was putting you in danger of forgetting to escape one parameter as a normal human being. and all it takes for an attacker to corrupt your system is just 1 unescaped parameter.

---

Changing The Code

typically changing the source files is always risky and has pain, especially if your software design is bad and if you don't have an obvious testing plan. but I will tell you what I did to make it as easier as possible.

I made a function that every database interaction code is going to use, so you can change what you want later in one place -that function- you can make something like this

class SystemModel
{
    /**
     * @param string $query
     * @param string $types
     * @param array $vars
     * @param mysqli $conn
     * @return boolean|$stmt
     */
    public function preparedQuery($query,$types, array $vars, $conn)
    {
        if (count($vars) > 0) {
            $hasVars = true;
        }
        array_unshift($vars, $types);
        $stmt = $conn->prepare($query);
        if (! $stmt) {
            return false;
        }
        if (isset($hasVars)) {
            if (! call_user_func_array(array( $stmt, 'bind_param'), $this->refValues($vars))) {
                return false;
            }
        }
        $stmt->execute();
        return $stmt;
    }

    /* used only inside preparedQuery */
    /* code taken from: https://stackoverflow.com/a/13572647/5407848 */
    protected function refValues($arr)
    {
        if (strnatcmp(phpversion(), '5.3') >= 0) {
            $refs = array();
            foreach ($arr as $key => $value)
                $refs[$key] = &$arr[$key];
                return $refs;
        }
        return $arr;
    }
}

Now, you can use this interface anywhere you want in your source files, for example let's change your current SQL statements you have provided in the question. Let us change this

$mysqli = new mysqli('localhost', "root", "", "testdb");
$addresult = "
                SELECT a.firstnames, a.surname, a.schoolrole, a.datejoined 
                FROM teachers a LEFT JOIN schools b ON a.schoolid = b.id 
                WHERE b.id = '".$inputvalues['schoolid']."'";

if( $result = $mysqli->query($addresult) ) {
    while($row = $result->fetch_all())
    {
        $returnResult = $row;
    }
}

Into this

$mysqli = new mysqli('localhost', "root", "", "testdb");
$sysModel = new SystemModel();
$addresult = "
                SELECT a.firstnames, a.surname, a.schoolrole, a.datejoined
                FROM teachers a LEFT JOIN schools b ON a.schoolid = b.id
                WHERE b.id = ?";
$types = "i"; // for more information on paramters types, please check :
//https://php.net/manual/en/mysqli-stmt.bind-param.php
$vars = [];
$vars[] = $inputvalues['schoolid'];

$stmt = $sysModel->preparedQuery($addresult, $types, $vars, $mysqli);
if (!$stmt || $stmt->errno) {
   die('error'); // TODO: change later for a better illustrative output
}
$result = $stmt->get_result();
$returnResult = [];
while ($row = $result->fetch_array(MYSQLI_ASSOC)) {
    $returnResult[] = $row;
}

Also, If I am not entering any data to the DB, is it still vulnerable to injection?

Yes, Sql Injection attack is applied by concatenating bad string to your SQL statement. whither it is an INSERT, SELECT, DELETE, UPDATE. for example

$query = "SELECT * FROM user WHERE name = '{$_GET['name']}' AND password = '{$_GET['pass']}'"

something like that could be exploited by

// exmaple.com?name=me&pass=1' OR 1=1; -- 

which will result in a SQL statement

$query = "SELECT * FROM user WHERE name = 'me' AND password = '1' OR 1=1; -- '"
//executing the SQL statement and getting the result
if($result->num_rows){
    //user is authentic
}else{
    //wrong password
}
// that SQL will always get results from the table which will be considered a correct password

Good luck with switching your software to prepared statements, and remember that the peace of mind you are going to get from knowing that whatever happens, you are safe from SQL injection attacks is worth the cost of changing the source files