Switching mySQL insert and update statements to PDO prepared statements to prevent SQL injection

Question

I'm trying to switch these mySQl INSERT INTO and Update statements to PDO prepared statements (primarily to prevent SQL Injection), but I'm having some difficulty getting the syntax right.

I’m currently using 2 types of INSERT/Update statements:

Statement 1 - Names Are Hardcoded

$qry = "INSERT INTO customer_info(fname, lname, email, user_name, password)
VALUES('$_POST[fname]','$_POST[lname]','$_POST[email]','$user_name','".sha1($salt + $_POST['password'])."')"; 
$result = @mysql_query($qry)

Statement 2 - Adding Names Dynamically

Instead of listing every element's name, most names are added dynamically (names are referenced as either $fieldlist or $setlist, and values are $vallist). The only names/values which are hardcoded are user_id or those which are arrays. I've included the full code for this below.

$result = mysql_query('UPDATE fit_table  SET '.$setlist.' WHERE user_id='.$user_id);
if (mysql_affected_rows()==0) {
$result = mysql_query('INSERT INTO fit_table ('.$fieldlist.') VALUES ('.$vallist.')'); };   

This is what I've tried:

Statement 1 - Based on this post https://stackoverflow.com/a/60530/1056713

$stmt = $conn->prepare("INSERT INTO customer_info VALUES(:fname, :lname, :email, :user_name, :password)");
$stmt->bindValue(':fname', $fname);
$stmt->bindValue(':lname', $lname);
$stmt->bindValue(':email', $email);
$stmt->bindValue(':user_name', $user_name);
$stmt->bindValue(':password ', $password);
$stmt->execute();

Statement 2 - Based on this PDO wrapper https://github.com/Xeoncross/DByte/blob/master/DB.php (referenced in this post https://stackoverflow.com/a/12500462/1056713 )

static function insert($fit_table, array $fieldlist){
$query = "INSERT INTO`$fit_table`(`" . implode('`,`', array_keys('.$fieldlist.')). '`) 
VALUES(' . rtrim(str_repeat('?,', count($fieldlist = array_values('.$vallist.'))), ',') . ')';
return DB::$p
? DB::column($query . 'RETURNING` user_id `', $fieldlist)
: (DB::query($query, $fieldlist) ? static::$c->lastInsertId() : NULL);
}

---

The full code for Statement 2 (This is how names are added dynamically at present)

// INSERT    
$fieldlist=$vallist='';
foreach ($_POST as $key => $value) {
    if ($key=='pants_waistband'){$value= implode(',',$value);}        
    $fieldlist.=$key.',';
    $vallist.='\''.($value).'\',';
}
$fieldlist=substr($fieldlist, 0, -1);
$vallist=substr($vallist, 0, -1);
$fieldlist.=', user_id';
$vallist.=','.$user_id;
// UPDATE
$setlist='';
foreach ($_POST as $key => $value) {
    if ($key=='pants_waistband'){$value= implode(',',$value);}  
    $setlist.=$key .'=\''.$value.'\',';
}
$setlist=substr($setlist, 0, -1); 

$result = mysql_query('UPDATE fit_table SET '.$setlist.' WHERE user_id='.$user_id);
if (mysql_affected_rows()==0) {
$result = mysql_query('INSERT INTO fit_table ('.$fieldlist.') VALUES ('.$vallist.')');}  

Answer 1

Look, whitelisting is not as boring as it seems!
Dynamic queries are great, and there is no reason to abandon the idea.
At least you can make it semi-dynamic, to avoid all these repetitions.

There is one great thing about PDO - it can accept an array with values, making repeated binding unnecessary. It can be as simple as

$stmt = $conn->prepare('INSERT INTO customer_info VALUES(?,?,?,?,?)');
$stmt->execute($_POST);

and it will be executed, if $_POST contain the exact number of fields in the right order. But as soon as we'll need field names in the query, it will either lose all the automation (as in your own answer) or become unsafe (as in your earlier dynamic code).

Well, let's make it both safe and dynamic.
The only thing you need is an array with allowed field names that would be our white list.
Then you can use this array to loop over $_POST, creating the query dynamically.
Here is a function to automate the process:
It takes three arguments but actually uses only one

function pdoSet($fields, &$values, $source = array()) {
  $set = '';
  $values = array();
  if (!$source) $source = &$_POST;
  foreach ($fields as $field) {
    if (isset($source[$field])) {
      $set.="`$field`=:$field, ";
      $values[$field] = $source[$field];
    }
  }
  return substr($set, 0, -2); 
}

It will return the string that looks like

`field1`=?,`field2`=?,`field3`=?

and will populate $values array for the use with PDO query.

Please note that Mysql allows SET syntax for either INSERT and UPDATE queries - no need for the VALUES syntax. So, one function for both kinds.

For the insert it would be as easy as

$fields = array("fname", "lname", "email", "user_name");
$stmt = $dbh->prepare("UPDATE users SET ".pdoSet($fields,$values));
$stmt->execute($values);

And it will remain the same 3-liner for whatever number of fields!

For the update it takes a little longer code. We will need to add some conditions to the query, as well as adding another member to the $values array.

$fields = array("fname", "lname", "email", "user_name");
$stmt = $dbh->prepare("UPDATE users SET ".pdoSet($fields,$values)." WHERE id = :id");
$values["id"] = $_POST['id'];
$stmt->execute($values);

The only question remains is how to add custom fields that aren't in the $_POST array yet.
I am just adding them there directly, before preparing:

$_POST['password'] = sha1($_POST['email'].$_POST['password']);

Hope this is what you were asking for.

Just one thing to clarify.
Prepared statements are not enough to stop an injection, and your case is a perfect example. They deal only with data, but to protect field names is your burden. Yet there is nothing wrong with old mysql way you were using. Your code just lacks the same whitelisting (and proper data formatting, of course). But if added, it will make your mysql query as safe as PDO.

Answer 2

I've discovered that white listing names is far more secure than other methods (thanks to the help of several people, including @zerkms), and wanted to share the finished statement.

It's now working properly, and includes the method required for connecting to the database with PDO. I also switched the db user account being used in the statement because I learned that it's best to use an account with limited privileges (that can only SELECT, INSERT, and UPDATE) to minimize the damage that a hacker could do.

try { 
      $conn = new PDO('mysql:host=localhost;dbname=dbname', 'Username', 'MyPassword');
      $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

      $stmt = $conn->prepare('INSERT INTO customer_info (fname...) VALUES(:fname...)');
      $stmt->bindParam(':fname', $_POST['fname'], PDO::PARAM_STR);
      $stmt->execute();   
    } catch(PDOException $e) {
  echo $e->getMessage();
}