0

I would like to secure my requests in my code.

Today my curent functions are like this.

public function UpdatePMS($table,$data,$where) {
    $ret        = array();
    $set_data   = "";
    foreach($data as $key => $value){
        $set_data .= $key."= '".$value."', ";
    }

    if (isset($where))      {
        $where = "WHERE ".$where;
    }

    $sql        = "UPDATE ".$table." SET ".$set_data."".$where;
    $sql        = str_replace(", WHERE", " WHERE", $sql);
    $stm        = $this->db->prepare($sql);
    $ret        = $stm->execute();
    return $ret;
}

With this way, i can select any tables, any datas, and any conditions. For example:

WHERE id = 1 and status < 10

Or only

WHERE id = 10 

Or sometimes

WHERE id = 1 and status >= 5

The content of where could change. A kind of universal request. Same for Delete, Update, Select, insert.

I tried to do like this, but it doesn't work.

$db     = new PDO('mysql:host=localhost;dbname=asterisk','root','');
$table = "my_table";
$where = "WHERE id = 1";
$sql    = 'SELECT * FROM :table :where';
$stm    = $db->prepare($sql);
$stm->execute(array(":table" => $table, ":where" => $where));
$ret    = $stm->fetchall(PDO::FETCH_ASSOC);

Any ideas?

Danard
  • 69
  • 8
  • 1
    **Table and Column names cannot be replaced by parameters in PDO.** – Masivuye Cokile Mar 27 '17 at 08:22
  • 1
    Prepared statement is not a magic wand that makes your query safe just by means of calling prepare(). There are RULES to follow. And yes, unfortunately, for the column and field names PDO offers no method to make them safe, you have to whitelist them manually. – Your Common Sense Mar 27 '17 at 08:29
  • Ok so i need to make one function per different requests, right? – Danard Mar 27 '17 at 08:47

1 Answers1

1

Frankly, you cannot use prepared statements this way. There are rules to follow. So it just makes no sense to write something like this

$table = "my_table";
$where = "WHERE id = 1";
$sql    = 'SELECT * FROM :table :where';
$stm    = $db->prepare($sql);
$stm->execute(array(":table" => $table, ":where" => $where));

instead you should write this code

$sql = 'SELECT * FROM my_table WHERE id = ?';
$stm = $db->prepare($sql);
$stm->execute(array($id));

Besides, you cannot parameterize table and field names, so it's better to write them as is.

so i need to make one function per different requests, right?

Honestly - yes. It will spare you from A LOT of headaches.

public function UpdatePMS($data, $id)
{
    $data[] = $id;
    $sql = "UPDATE table SET f1 = ?, f2 = ? WHERE id = ?";
    $stm = $this->db->prepare($sql);
    $ret = $stm->execute($data);
    return $ret;
}

which is going to be used like

$obj->UpdatePMS([$f1, $f2], $id);
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345