PDO prepared statement - syntax and sanitization

Question

I'm trying to update my database with the following:

$fields = array(
    'titulo',
    'tipo_produto',
    'quantidade_peso',
    'unidade_de_venda',
    'unidades_por_caixa',
    'caixas_piso',
    'pisos_palete',
    'tipo_palete',
    'unidades_palete',
    'caixas_palete',
    'uni_diametro',
    'uni_largura',
    'uni_profundidade',
    'uni_altura',
    'caixa_largura',
    'caixa_profundidade',
    'caixa_altura',
    'altura_palete',
    'volume_unidade',
    'volume_caixa',
    'volume_palete',
    'peso_caixa',
    'peso_palete'
);
$sql = 'UPDATE ficha_item SET '.implode(', ', array_map(create_function('$value', 'return "$value=\"" . $_POST["$value"] ."\"";'), $fields)).' WHERE id=?';

$stmt = $db->prepare($sql);

$stmt->execute(array($_POST['item_id']));
$stmt->closeCursor();

This seems to work pretty well, but I'm wondering about the security, is this sanitized at all?

I came up with this solution after attempting (with no success) another solution:

$fields = array(
    'titulo',
    'tipo_produto',
    'quantidade_peso',
    'unidade_de_venda',
    'unidades_por_caixa',
    'caixas_piso',
    'pisos_palete',
    'tipo_palete',
    'unidades_palete',
    'caixas_palete',
    'uni_diametro',
    'uni_largura',
    'uni_profundidade',
    'uni_altura',
    'caixa_largura',
    'caixa_profundidade',
    'caixa_altura',
    'altura_palete',
    'volume_unidade',
    'volume_caixa',
    'volume_palete',
    'peso_caixa',
    'peso_palete'
);
$sql = 'UPDATE ficha_item SET ? WHERE id=?';

$valuesClause = implode(', ', array_map(create_function('$value', 'return "$value=\"" . $_POST["$value"] ."\"";'), $fields));

$stmt = $db->prepare($sql);

$stmt->execute(array($valuesClause, $_POST['item_id']));
$stmt->closeCursor();

No errors at all, but my database would not be updated. Is my first solution sanitized at all? What went wrong with my original idea? I'm thinking it has something to do with how PDO sanitizes the query on the execute... but I'm out of ideas on where to go with it.

NOTE: The database column names and the input names are the same, that is why $value works. In case you're also wondering, anonymous functions are out of question due to live PHP version.

See [How to squeeze error message out of PDO?](http://stackoverflow.com/q/3726505) for how to get a meaningful error message. Daniel's answer is correct though, PDO's escaping is for data only, not column names. — Pekka, Jan 07 '13 at 11:37

score 2 · Answer 1 · edited May 23 '17 at 10:24

A prepared statement is a prepared statement, a statement argument/parameter is not treated as SQL for obvious reasons.

You need to prepare your query before you prepare the statement.

I suggest you read the PDO manual. You should also have a look at this question for clarification on how to protect your queries.

$columns = array();
foreach ($fields as $column)
    $columns[] = "$column = ?";

$sql = "UPDATE table SET " . implode(" AND ", $columns) . " WHERE id = ? ";

// Now you may prepare the statement

score 1 · Accepted Answer · answered Jan 07 '13 at 11:58

$fields = array(
    'titulo',
    'tipo_produto',
    'quantidade_peso',
    'unidade_de_venda',
    'unidades_por_caixa',
    'caixas_piso',
    'pisos_palete',
    'tipo_palete',
    'unidades_palete',
    'caixas_palete',
    'uni_diametro',
    'uni_largura',
    'uni_profundidade',
    'uni_altura',
    'caixa_largura',
    'caixa_profundidade',
    'caixa_altura',
    'altura_palete',
    'volume_unidade',
    'volume_caixa',
    'volume_palete',
    'peso_caixa',
    'peso_palete'
);
$sql="UPDATE ficha_item SET ".implode(", ",array_map(function($s){
    return "$s = ?";
},$fields))." WHERE id=?";
print_r($sql);

Outputs:

UPDATE ficha_item SET titulo = ?, tipo_produto = ?, quantidade_peso = ?, unidade_de_venda = ?, unidades_por_caixa = ?, caixas_piso = ?, pisos_palete = ?, tipo_palete = ?, unidades_palete = ?, caixas_palete = ?, uni_diametro = ?, uni_largura = ?, uni_profundidade = ?, uni_altura = ?, caixa_largura = ?, caixa_profundidade = ?, caixa_altura = ?, altura_palete = ?, volume_unidade = ?, volume_caixa = ?, volume_palete = ?, peso_caixa = ?, peso_palete = ? WHERE id=?

This is the SQL statement that you need to prepare. Now to get your prepared statement:

$stmt=$db->prepare($sql);
$stmt->execute(array_merge(array_map(function($s){
    return $_POST[$s];
},$fields),array($_POST["item_id"])));

No database for me to test, but this should be the right track.

Note that I'm just following your "style"; Even though this code is treating all $_POST as parameters and thus avoiding SQL injection, it's assuming that every $_POST[$fields[$idx]] exists, which can be easily broken by any user, so this is where you should sanitize.

Edit:

Since you've updated that you can't use anonymous function, you can construct the required arrays manually:

$cache=array();
foreach($fields as $field)
    $cache[]="$field = ?";
$sql="UPDATE ficha_item SET ".implode(", ",$cache)." WHERE id=?";

$cache=array();
foreach($fields as $field)
    $cache[]=isset($_POST[$field])?$_POST[$field]:null;
$cache[]=$_POST["item_id"]
/*...*/
$stmt->execute($cache);

score 1 · Answer 3 · edited May 23 '17 at 11:55

You've got good aim
When your fields are whitelisted in your code, it's enough for the protection (as a matter of fact. whitelisting is the only proper solution here).

So, the only thing you need is to get a correct SQL out of your white list and $_POST array.
Voila - here is a useful function written for this very purpose.
with it your code going to be

$sql  = "UPDATE ficha_item SET ".pdoSet($fields,$values)." WHERE id = :id";
$stmt = $db->prepare($sql);
$values["id"] = $_POST['id'];
$stmt->execute($values);

(User-defined) functions are great. Dunno why no one using them.

PDO prepared statement - syntax and sanitization

3 Answers3

Linked