Switching to Prepared Statements

Question 1

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: %sn", 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.

Question 2

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://.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

Question 3

I would say just build it yourself. You can set it up like this:

$query = "INSERT INTO x (a,b,c) VALUES ";
foreach ($arr as $item) {
  $query .= "('".$item[0]."','".$item[1]."','".$item[2]."'),";
}
$query = rtrim($query,",");//remove the extra comma
//execute query

Don't forget to escape quotes if it's necessary.

Also, be careful that there's not too much data being sent at once. You may have to execute it in chunks instead of all at once.

Question 4

The function you're looking for is find_in_set:

 select * from ... where find_in_set($word, pets)

for multi-word queries you'll need to test each word and AND (or OR) the tests:

  where find_in_set($word1, pets) AND find_in_set($word2, pets) etc

Question 5

The main mistake in your code was the typo with '?' in your query. If the ? is inside quotes it is not treated as a placeholder, but as a literal value.

When using MySQLi you should enable MySQLi exception mode. If you do then there is no longer any need to check the result of each function. You should also use OOP style as it is less verbose and you are less likely to make a silly mistake.

// put this line before you open the MySQLi connection
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);

$stmt = $linkDB->prepare('SELECT * FROM '.PREFIX.'Issues WHERE id=? AND disabled=0 LIMIT 1');
$stmt->bind_param('i', $_POST['article']);
$stmt->execute();
$detail = $stmt->get_result()->fetch_assoc();

Question 6

Not having received any feedback in the form of answers or even a comment, I was getting ready to give up, when I stumbled onto this excellent blog post:

How to bind custom Hibernate parameter types to JPA queries

The post gives two options for controlling the types which JPA passes through the driver to Postgres (or whatever the underlying database actually is). I went with the approach using TypedParameterValue. Here is what my code looks like continuing with the example given above:

Query query = entityManager.createNativeQuery(sqlString, MyEntity.class);
query.setParameter("param1", new TypedParameterValue(StringType.INSTANCE, null));
query.setParameter("param2", new TypedParameterValue(DoubleType.INSTANCE, null));
List<MyEntity> = query.getResultList();

Of course, it is trivial to be passing null for every parameter in the query, but I am doing this mainly to show the syntax for the text and double columns. In practice, we would expect at least a few of the parameters to be non null, but the above syntax handles all values, null or otherwise.

Switching to Prepared Statements

Answers

Changing The Code

php,mysql