2010-07-21 5 views
0

J'ai la fonction d'insertion suivante. Est-il sûr d'une injection de sql. Si ce n'est pas le cas, comment puis-je le rendre sûr?Cette fonction PDO est-elle sûre de l'injection sql?

public function insert($postValues, $table){ 

    $dbh = $this->connect(); 

    try { 
     $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); 

     $fields = implode(array_keys($postValues), ','); 
     $values = "'".implode(array_values($postValues), "','")."'"; 
     $insertQuery = 'INSERT INTO '.$table.' ('.$fields.') VALUES (:'.$fields.')'; 

     $stmt = $dbh->prepare($insertQuery); 

     foreach($postValues as $vals) { 
      $stmt->execute($vals); 
     } 

     $message = $sucessMessage; 
    } 
    catch(PDOException $e){ 
     $message = $e->getMessage(); 
    } 

    $dbh = null; 

    return $message; 
} 

Merci à l'avance

Répondre

2

Si chaque type de colonne est un PDO::PARAM_STR, il est assez simple de lier vos paramètres marqueurs unamed de paramter utilisant PDOStatement::execute. Toutefois, si les types de colonne varient, vous devez spécifier le type de colonne pour chaque colonne lorsque vous liez avec PDOStatement::bindParam. Accepter les noms de table et de colonne à partir de ce qui semble être une entrée utilisateur n'est pas une bonne idée. La requête échouera si les noms de table ou de colonne sont incorrects, mais vous devez être très prudent pour vous assurer que les noms de table et de colonne sont sûrs à utiliser. L'exemple suivant vérifie la table et les noms de colonnes contre une liste blanche, avant d'exécuter une SQL:

function insert($postValues, $table) { 
    $dbh = $this->connect(); 

    // Create a simple whitelist of table and column names. 
    $whitelist = array('my_table' => array('col1', 'col2', 'col3')); 

    // Check if the table name exists in the whitelist. 
    if(!array_key_exists($table, $whitelist)) { 
     exit("$table is not a valid table name.\n"); 
    } 

    // Count the number of columns that are found in the whitelist. 
    $cols = count(
     array_intersect(
      $whitelist[$table], 
      array_keys($postValues))); 

    if($cols !== count($postValues)) { 
     exit("One or more invalid column names have been supplied.\n"); 
    } 

    // Create a comma separated list of column names. 
    $columns = implode(', ', array_keys($postValues)); 
    // Create a comma separated list of unnamed placeholders. 
    $params = implode(', ', array_fill(0, count($postValues), '?')); 
    // Create a SQL statement. 
    $sql = "INSERT INTO $table ($columns) VALUES ($params)"; 

    // Prepare the SQL statement. 
    $stmt = $dbh->prepare($sql); 
    // Bind the values to the statement, and execute it. 
    return $stmt->execute(array_values($postValues)); 
} 

echo insert(
    array(
     'col1' => 'value1', 
     'col2' => 'value2', 
     'col3' => 'value3'), 
    'my_table'); 

// 1 

echo insert(
    array(
     'col1' => 'value1', 
     'col2' => 'value2', 
     'col3' => 'value3'), 
    'unsafe_table'); 

// unsafe_table is not a valid table name. 

echo insert(
    array(
     'col1' => 'value1', 
     'col2' => 'value2', 
     'unsafe_col' => 'value3'), 
    'my_table'); 

// One or more invalid column names have been supplied. 
-1

Non, parce que vous êtes juste exécution d'une requête SQL brute avec l'extension PDO. Je fais quelque chose de semblable au suivant:

$fields = array(); 
$values = array(); 

foreach ($_POST as $field => $value) { 
    $fields[] = $field; 
    $values[] = $this->pdo->quote($value); 
} 

$fields = implode(',', $fields); 
$values = implode(',', $values); 

$sql = "INSERT INTO $table ($fields) VALUES ($values)"; 
$res = $this->pdo->query($sql); 

Je suis sûr que vous pouvez modifier ce qui précède pour s'adapter à votre configuration.

+0

Mais, comme on peut le lire dans la documentation (http://php.net/manual/en/pdo.quote .php), vous feriez mieux d'utiliser prepare().Vous pouvez facilement créer dynamiquement une chaîne sql avec des espaces réservés et ensuite appeler bindParam/bindValue pour les lier à l'instruction. –

+0

Oui, en effet. Bien que si je boucle sur mes données '$ _POST' (ou n'importe quel tableau que j'utilise) je peux aussi agir sur des données. Par exemple, dictez les mots de passe avant qu'ils ne soient utilisés dans la requête ou la paire clé/valeur si je mets à jour et si l'utilisateur n'a rien entré comme nouveau mot de passe. Si je n'ai pas supprimé la clé, une chaîne vide sera hachée et interrompra la connexion de l'utilisateur. Juste un scénario à penser. –

+0

@Dennis: Malheureusement, vous ne pouvez pas facilement lier des valeurs de tableau à des instructions préparées. Donc, l'approche ci-dessus est OK. Bien sûr, vous pouvez créer dynamiquement l'instruction SQL préparée (en ajoutant autant d'espaces réservés que vous le souhaitez), puis lier chaque élément de tableau à cette instruction. –

1

Soit dit en passant: en demandant si PDO est plus sûr de l'injection sql que d'une autre bibliothèque de connexion PHP MySQL, la réponse est NON quand on parle de PDO_MYSQL (ne sais pas si ce qui suit est vrai pour d'autres bases de données).

On pourrait même soutenir l'inverse, AOP est moins sûr et plus dangereux que toute autre bibliothèque de connexion PHP MySQL (ext/mysql et ext/mysqli) car PDO_MYSQL permet de multiples requêtes dans une instruction SQL lors ext/mysql arrêts multi-requêtes complètement et ext/mysqli a une fonction de sparate mysqli_multi_query().

Je viens d'essayer de trouver des sources pour étayer cette affirmation, mais les seules choses que j'ai sont:

+2

C'est peut-être le cas, mais une fois que vous utilisez des instructions préparées (comme vous devriez le faire), ce problème cesse d'exister. –

+0

@Dennis: C'est exact ... –

4

La seule façon saine consiste à utiliser PDO::prepare avec des paramètres (voir exemple dans le manuel). De plus, les noms de champs doivent provenir d'une source fiable, c'est-à-dire ne pas être un utilisateur. De cette façon, vous construisez votre chaîne de requête à partir de composants de confiance:

function insert ($table, $fields, $data) 
{ 
    $field_names = implode (", ", $fields);      # "a, b" 
    $values = ":" . implode (", :", $fields);     # ":a, :b" 
    $query = "INSERT INTO $table($field_names) VALUES($values)"; 
    $sth = $pdo->prepare ($query); 

    foreach ($data as $row) { 
     # Here you can even remove "bad" keys from $row 
     $sth->execute ($row); 
    } 
} 

$fields = array ('a', 'b'); # those are hard-coded in application 
$data = array (   # those come from user 
    array ('a'=>'Apple', 'b'=>'Bean'), 
    array ('a'=>'Avocado', 'b'=>'Blueberry', '); DELETE FROM fruits; -- '=>'evil'), 
); 
insert ('fruits', $fields, $data); 
+0

La réponse de Mike est meilleure dans ce cas. L'instruction ci-dessus exécute autant de requêtes que de champs, qui peuvent aller de centaines à une seule requête INSERT. – JM4

Questions connexes