Security when creating a PDO query function

Question

I am now migrating to PDO because of security, but I don’t know if I am using PDO correctly.

I am creating these functions to make faster the use of PDO, but I do not know if this form is safe. It’s the same thing that I’m using the function-free form?

<?php 
    function Conecta(){
try{
    $conexao=new PDO("mysql:host=localhost;dbname=sistemaescolar",'root','');
}
catch(PDOException $erro){
    $erro->getMessage();
}   
return  $conexao;
}

function ExecutaConsulta($conexao,$sql){
  $valor=func_get_args();
  $valores=array_slice($valor,2);
  $preparado=$conexao->prepare($sql);
  $preparado->execute($valores);
  $executar=$preparado->fetch();

  return $executar;

}
$conecta=Conecta();


$sql="SELECT * FROM se_professores WHERE id=?";
$resultado=ExecutaConsulta($conecta,$sql,1);
echo $resultado['nome'];

?>

Answer 1

Code analysis

Its implementation is correct, because by passing the values of a query as an array to the function Pdostatement::execute, it will prepare the received parameters treating all as PDO::PARAM_STR, that is, all received parameters will be treated as a string and will not risk suffering an Sql Injection, unless the values are concatenated earlier in your query.

In the example below, I use an array as a third parameter, but the reason will be explained later.

 // Forma vulnerável
 $nome = $_POST['nome'];
 $valor = $_POST['valor'];
 $query = 'INSERT INTO REGISTRY (name, value) VALUES (' . $nome . ', ' . $valor . ')';
 $resultado0 = executaConsulta($conecta, $query);

 // 1º Forma correta
 $query = 'INSERT INTO REGISTRY (name, value) VALUES (:name, :value)';
 $valores = array(
      ':name' => $_POST['nome']
      ':value' => $_POST['value']
 );
 $resultado1 = executaConsulta($conecta, $query, $valores);

 // 2º Forma correta
 $query = 'INSERT INTO REGISTRY (name, value) VALUES (?, ?)';
 $valores = array(
      $_POST['nome']
      $_POST['value']
 );
 $resultado2 = executaConsulta($conecta, $query, $valores);

 // 3º Forma correta
 // A função filter_input é utilizada para acessar variáveis globais de forma mais segura.
 // Existem outras funções deste tipo. http://php.net/manual/en/book.filter.php
 $query = 'INSERT INTO REGISTRY (name, value) VALUES (:name, :value)';
 $valores = array(
      ':name' => filter_input( INPUT_POST, 'nome', FILTER_SANITIZE_STRING );
      ':value' => filter_input( INPUT_POST, 'value', FILTER_SANITIZE_STRING );
 );
 $resultado3 = executaConsulta($conecta, $query, $valores);

About prepared query or Prepared statements

Some databases allow to use a resource of reuse of queries. It would be something like preparing a text where only a few words change, and each time you use this text, you need to inform only what the new words are.

However, not all databases support this feature (today is no longer the case with Mysql), and as the PDO aims to be a practical interface for using multiple database drivers, they had to implement a way that this feature could be used when needed. To solve this problem, the resource is simulated by the function.

In the documentation there are these examples:

$stmt = $dbh->prepare("INSERT INTO REGISTRY (name, value) VALUES (:name, :value)");
$stmt->bindParam(':name', $name);
$stmt->bindParam(':value', $value);

// Inserindo uma linha
$name = 'one';
$value = 1;
$stmt->execute();

// Inserindo outra linha com diferentes valores
$name = 'two';
$value = 2;
$stmt->execute();

Forcing PDO to use native prepared queries

To force PDO to always use the native prepared queries feature, add right after connecting:

$conexao->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

The PDO has treatment for if an error occurs, the emulated version is used, but there are some disadvantages in using the native feature in some versions

• Do not take advantage of cache resulting in loss of performance;
• Cannot be performed by some types of queries like "SHOW TABLES";
• Do not communicate well with the size of columns in other queries that use "SHOW" returning distorted results.
• Calling several times procedurespode cause connection loss.

Some additional information on https://stackoverflow.com/questions/10113562/pdo-mysql-use-pdoattr-emulate-prepares-or-not

Binding of values and variables

In PDO there are two methods that allow you to associate parameters to your queries, they are the Pdostatement::bindParam and Pdostatement::bindValue.

The bindParam associates the name of a variable to the query parameter, that is, if the value of a variable is changed and the method execute is called again, the query will be executed with the new value.

$stmt = $dbh->prepare("INSERT INTO REGISTRY (name) VALUES (:name)");
$stmt->bindParam(':name', $name);

// Inserindo uma linha
$name = 'one';
$stmt->execute();

// Inserindo outra linha com diferentes valores
$name = 'two';
$stmt->execute();

Already the bindValue assigns a specific value to a parameter of your query, ie to change the value of the parameter you will have to call the method bindValue again.

$stmt = $dbh->prepare("INSERT INTO REGISTRY (name) VALUES (:name)");

// Inserindo uma linha
$stmt->bindValue(':name', 'one');
$stmt->execute();

// Inserindo outra linha com diferentes valores
$stmt->bindValue(':name', 'two');
$stmt->execute();

In both methods it is possible to specify which type should be used to treat the query parameter value. O bindParam besides the type it is also possible to indicate other options such as the maximum value size.

Suggestions for improvements

Improvement in code style

I recommend giving a read on the attempt that the PHP community has made to standardize some things, the PSR-2 is a guide on how to better format your code. Improving this template is a good point to start observing what your code can improve and will help you find both security holes and some bugs.

Organization of parameters

When using the parameters "magic" using the function func_get_args() you will have problems documenting, because the signature of your function does not make it explicit that it is possible to pass more arguments, this will leave other developers confused with your code.

In the presented code, there is no need for the function to receive infinite parameters, because the only thing that the function code performs is to obtain an array of everything that has been passed, removing what is already known.

To make the signature clearer, you can set a third parameter with a default value, in the case of an empty array, because the function execute PDO expects an array.

// Versão reescrita
function executaConsulta($conexao, $sql, $valores = array()){
    $preparado = $conexao->prepare($sql);
    $preparado->execute($valores);
    $resultado = $preparado->fetchAll();

    return $resultado;
}

In the function I also changed the function Dostatement::fetch() by function Dostatement::fetchAll(), so its function will always return all the result obtained in an array. Working with array is much easier than with PDO objects.

Use of the function

When using the function, you will make it clearer to use keys instead of the wildcard character (?) as well as avoid having to pass the same value twice, if used in two places of your query. See the two possibilities of use:

// 1º forma
$sql = "SELECT * FROM se_professores WHERE id = ?";
$resultado1 = executaConsulta($conecta, $sql, array(1));

foreach ($resultado1 as $linha) {
    echo $linha['nome'];
}

// 2º forma
$sql = "SELECT * FROM se_professores WHERE id = :id";
$valores = array(':id' => 1);
$resultado2 = executaConsulta($conecta, $sql, $valores);

foreach ($resultado1 as $linha) {
    echo $linha['nome'];
}

Answer 2

Look it all depends on how ExecutaConsulta will be used. If as in your example:

$sql="SELECT * FROM se_professores WHERE id=?";
$resultado=ExecutaConsulta($conecta,$sql,1);

there is no obvious security problem, however nothing in your code prevents calls of this type:

$userId = "1; DROP TABLE se_professores"; // user input com evil SQL Injection
$sql="SELECT * FROM se_professores WHERE $userId";
$resultado=ExecutaConsulta($conecta,$sql);

So the answer is that there is nothing wrong with its function and at the same time it does not entail a "safe use" of PDO.

Another thing, maybe you’re just using for testing the new PDO("mysql:host=localhost;dbname=sistemaescolar",'root',''); but anyway it’s always good to remember, never use the root user of the database in production and never drop a user without password in the production database.

Answer 3

The best (and most recommended) way is the correct use of PDO. Although it is a little more difficult to implement, it ensures greater security when executing the query. That’s because the process is done in two parts.

1) We created the query template. This template is nothing more than a template of what the query will be. Unlike the other methods, you will not enter the variable itself at this time. It will only indicate where the variable will be placed. In case, it will be right after the "...name =".

$var = $pdo->prepare('SELECT * FROM tabela where nome = :nome');

2) Now we indicate which value will occupy the space indicated by ":name":

$var->bindValue(':nome', 'Nome da pessoa');

At the end the code will look something like this:

<?php
$pdo = new PDO('mysql:host=localhost;dbname=crud', 'root', '');
$var = $pdo->prepare('SELECT * FROM tabela where nome = :nome');
$var->bindValue(':nome', 'Nome da pessoa');
$run = $var->execute();
$result = $var->fetchAll(PDO::FETCH_ASSOC);
var_dump($result);
?>

There is also another way. A faster way (but not the most indicated) is to remove special characters from the variables that will be used in the query. Without using ";" (semicolon) and "'" (single or double quotes) and other special characters it is practically impossible to execute sql Injection.

Take the example:

$password = preg_replace('/[^[:alnum:]_]/', '',$senha);

If you want more information:> http://www.devmedia.com.br/evitando-sql-injection-em-aplicacoes-php/27804