Converting query to parametrised query

Question

I have a query on my site and have recently been hacked because of it.

I have spent a good 2 hours looking how to convert this query so it is secure and have not got anywhere.

If anyone don't mind, could you please convert this one for me just so I can see what to do on the rest?

$camera_id = $_GET['camera_id'];

$cameras = mysqli_query($conn, "SELECT * FROM cameras WHERE id = $camera_id");
$camera = mysqli_fetch_array($cameras);

investigate the use of PDO and using bound parameters instead of the id of the $_GET query — gavgrif, Nov 13 '16 at 00:50
I would normally downvote these sort of broad questions but since you are making an effort to learn the proper way, I thought I would answer instead — e4c5, Nov 13 '16 at 01:00
The manuals on both mysqli and pdo prepared statements, contain clear examples on their use and syntax. You should have posted what you tried instead. — Funk Forty Niner, Nov 13 '16 at 01:42
Possible duplicate of [How can I prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) — Funk Forty Niner, Nov 13 '16 at 01:48

e4c5 · Accepted Answer · 2016-11-13T01:25:35.063

1

Try something like this.

$camera_id = $_GET['camera_id'];

$cameras = mysqli_prepare($conn, "SELECT * FROM cameras WHERE id = ?");
mysqli_stmt_bind_param($cameras, $camera_id);
$cameras->execute();

While you are making the switch, switch straight away to PDO. It's far better than mysqli

   $db = new PDO('mysql:host=localhost;dbname=mydb', 'username', 'password');
   $stmt = $db->prepare("SELECT * FROM cameras WHERE id = :camera_id");
   $stmt->execute(array(":camera_id"=>$camera_id));
   $result = $stmt->fetchAll();

or instead of fetchAll()

while($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
    echo $row['field1'].' '.$row['field2']; //etc...
}

As you can see this is more readable. And if you later decide eto switch to postgresql the change is real easy.

edited Nov 13 '16 at 01:25

answered Nov 13 '16 at 00:57

e4c5

52,766
11
101
134

Hello, thank you very much for your reply, I have firstly tried the top one and get this error : Fatal error: Call to undefined function mysqli_bind_param() in category_page.php on line 12 – Bradley Nov 13 '16 at 01:01
sorry @e4c5 - typo in stmt..... a double quote.... :) – gavgrif Nov 13 '16 at 01:09
Thanks, I have added this and it now works, I just get thrown this error : – Bradley Nov 13 '16 at 01:10
Warning: mysqli_stmt_bind_param(): Number of elements in type definition string doesn't match number of bind variables in category_page.php on line 11 – Bradley Nov 13 '16 at 01:11
@Bradley Switch right away to PDO. PDO is easier to learn (& write) and far better. – Charlotte Dunois Nov 13 '16 at 01:12
Okay, I have switched it all to PDO and it works great thank you, im just wondering, can I have the results looped in a while look like I had this before ? – Bradley Nov 13 '16 at 01:22
while($camera = mysqli_fetch_array($cameras)) { – Bradley Nov 13 '16 at 01:22
yes the fetchall() gives you something you can loop through, or see the update – e4c5 Nov 13 '16 at 01:25
Thankyou for your help @e4c5, site all updated and working with PDO :) – Bradley Nov 13 '16 at 09:07
1

Glad to have been of help. All the best for your project. – e4c5 Nov 13 '16 at 09:25

gavgrif · Answer 2 · 2016-11-13T01:06:02.090

This is using PDO and assumes that the camera id is a number (if it can contain non-numerical values swap the PARAM_INT for a PARAM_STR. The basic premise is that you separate the query from the variables and you bind the value of the desired item to a variable. Also note that you would need to alter the variables in the new PDO declaration to suit your own database. Note also that fetchAll() provides an associative array of the returned results - there are a number of other fetch() methods possible to give different outcomes - look for the official documentation.

$camera_id = $_GET['camera_id'];

$conn = new PDO('mysql:host=localhost;dbname=db', 'username', 'password');

$sql = "SELECT * from cameras where id = :cameraId";
$q = $conn->prepare($sql);
$q -> bindValue(":cameraId" , $camera_id, PDO::PARAM_INT);
$q->execute();
$cameraRows = $q->fetchAll();
  foreach($cameraRows as $cameraRow){
    $CID= $cameraRow["camera_id"];
    //.... rest of the code
  }

Converting query to parametrised query

2 Answers2