Understanding Prepared Statements - PHP

Question

I am working on a school project for the finacial aid office at a university. The project is in production and have most of it done apart from a few little tweaks here and there. My main concern over the winter break (now) is security and preventing any breaches to the best of my abilities. People have told me to steer into Prepared Statements. I understand them to a good extent except for inserting data.

I have two forms : a login in form and student login form. The student login form enters why a student is coming to the office. that form is then submitted and that data is later retrieved by a table that shows counselors what students are waiting to be seen.

My problem is though each student who walks into the financial aid office has his or her own unique problem (most of the time) so now what confuses me is :

Do I need to think ahead and pre-make the insert queries or is there a way for there to be a "dynamic" query because there is a student comments box and for that it will be totally unique so how will I be able to create a query for that?

<?php
define('DB_Name', 'dbtest');
define('DB_User', 'root');
define('DB_Password', 'testdbpass');
define('DB_Host', 'localhost');

$link = mysql_connect(DB_Host, DB_User, DB_Password);

if (!$link) {
  die ('Could Not Connect: ' . mysql_error ());
}

$db_selected = mysql_select_db(DB_Name, $link);

if (!db_selected) {
  die('Can Not Use ' . DB_name . ': ' . mysql_error());
}

$value1 = $_POST ['anum'];
$value2 = $_POST ['first'];
$value3 = $_POST ['last'];
$value4 = $_POST ['why'];
$value5 = $_POST ['comments'];

$sql = "INSERT INTO `dbfinaid` (anum, first, last, why, comments) VALUES ('$value1', '$value2', '$value3', '$value4', '$value5')";

if (!mysql_query($sql)) {
  die('Error : ' . mysql_error());
}

mysql_close();

and as I have been told doing it that way leaves me prone to SQL-Injections.

Any help will be very much appreciated. Thank you.

Obviously prepared statements can be dynamic. You just read this: http://wiki.hashphp.org/PDO_Tutorial_for_MySQL_Developers And that's it — gd1, Dec 21 '12 at 17:12
The pastebin doesn't seem to be loading. Can you place pertinent code in your question? — Mike Brant, Dec 21 '12 at 17:13
@MikeBrant: loads for me. However, it is a classical mysql_query() approach prone to SQL-Injections :) — gd1, Dec 21 '12 at 17:13
See this question: http://stackoverflow.com/questions/60174/how-to-prevent-sql-injection — bfavaretto, Dec 21 '12 at 17:18
My suggestion would be to rework your `mysql_*` operation into `mysqli_*` operations, as you REALLY should not be using `mysql_`. Especially if this is for a class project, you don't want to be showing your professor how you can't read the big red warnings on PHP.net about those functions being deprecated. MySQLi php client can be just as prone to injections as MySQL PHP client if you are not using them correctly, so don't think that just changing from one to the other is a magic bullet. Here is a starting point on using prepared statments: http://php.net/manual/en/mysqli.prepare.php — Mike Brant, Dec 21 '12 at 17:19
@gd1: Prepared statements can be dynamic, sure...but *making* them dynamic sacrifices nearly every single benefit of prepared statements. Once you switch the table based on what the user picks, the query has to be recompiled, plus you're back to having to make sure that user input never makes it unescaped into the SQL. (Why's the table changing? User input, right? :P) — cHao, Dec 21 '12 at 17:25
Just as standard mysql functions are not inherently insecure... if you use them properly. — Mattt, Dec 21 '12 at 17:27
yes i understand that, its just in this situation the project calls for a "comments box" so that each student can add input on their own special situation. Not all students will have the same reason for coming to the office. So I need to have the comments box be unique in almost every input. I will start reading a lot more as I was only looking at tutorials online. I didn't even think of looking at php.net thanks. Any more insight to this would be lovely. Thanks, RaGe — RaGe10940, Dec 21 '12 at 17:27
@RaGe10940: And having a `more_info` column wouldn't work because...? — cHao, Dec 21 '12 at 17:28
If you do decide to go with rewriting your code for lets say `mysqli` I would strongly suggest to go for `PDO` instead. Indeed PDO still has some quirks but its far supperior to `mysqli` especially when working with stuff like prepared statements etc. — , Dec 21 '12 at 17:29
@holodoc: Superior, my ass. :) mysqli supports the features of mysql that PDO avoids because it's trying to stay abstract. It's just *different*. — cHao, Dec 21 '12 at 17:30
I used to prefer PDO but I am reconsidering after I realized that it enables multi-statements *by default*. This is a huge SQL injection vulnerability. :-( — Bill Karwin, Dec 21 '12 at 17:40
@cHao The way the form is structured is like so : a student enters id number (each student has one) first name last name and then there is a drop down menu that has generic topics like : (loan problem, state aid problem *usa*, administrator appeal, drop off documents and finally other) now if the student chooses others he or she is trying to tell us that the situations listed in the drop down menu are not going to corresponding with their individual needs. So i added a comments box for the student to give a brief summary of their problem. — RaGe10940, Dec 21 '12 at 17:56
Wow, if this is deployed you're in serious trouble. I hope you can fix this **immediately**. — tadman, Dec 21 '12 at 18:27
I have 16 weeks until the project is due. That is why I am working on it now. I am thankful for your brutal comments as that just shows how cruddy the coding was. Again thank you and yes I will get right to rewriting all my code with proper SQL-Injection counter measures. — RaGe10940, Dec 21 '12 at 19:11
@cHao Yes PDO can sometimes feel a bit too abstract however every time I need to spam my code with calls to `call_user_func_array` just in order to use result binding in conjunction with dynamically generated queries I remember why I like PDO. — , Dec 21 '12 at 19:21

maček · Answer 1 · 2012-12-21T17:25:06.677

Once you read up on PHP's PDO you can rewrite your code like this

$dbh = new PDO('mysql:host=localhost;dbname=dbtest', $user, $pass);

try {
  $query = $dbh->prepare("INSERT INTO `dbfinaid` (anum, first, last, why, comments) VALUES (:anum, :first, :last, :why, :comments)");

  $query->bindParam(':anum',     $_POST['anum'],     PDO::PARAM_INT);
  $query->bindParam(':first',    $_POST['first'],    PDO::PARAM_STR);
  $query->bindParam(':last',     $_POST['last'],     PDO::PARAM_STR);
  $query->bindParam(':why',      $_POST['why'],      PDO::PARAM_STR);
  $query->bindParam(':comments', $_POST['comments'], PDO::PARAM_STR);

  $query->execute();
}
catch (PDOException $e) {
  die("error occured:" . $e->getMessage());
}

score 5 · Accepted Answer · edited Apr 04 '14 at 05:53

5

Building on the answer from @maček, here's an alternative way of doing the same thing. I find this easier:

$dbh = new PDO('mysql:host=localhost;dbname=dbtest', $user, $pass);

try {
  $query = $dbh->prepare("INSERT INTO `dbfinaid` (anum, first, last, why, comments)
    VALUES (:anum, :first, :last, :why, :comments)");

  $params = array_intersect_key($_POST, array_flip(array('anum', 'first', 'last', 'why', 'comments')));
  $query->execute($params);
}
catch (PDOException $e) {
  error_log($e->getMessage());
  die("An error occurred, contact the site administrator.");
}

I prefer to output the SQL error to a log, and show a different error to the user that doesn't confuse them with code details.

edited Apr 04 '14 at 05:53

maček

76,434
37
167
198

answered Dec 21 '12 at 17:28

Bill Karwin

538,548
86
673
828

so macek and your code will allow for the "comments" box to be unique per each student and it will prevent attacks such as SQL-Injections? I have seen both of these examples on tutorials i just didn't understand if doing what the both of you have prescribed will work against attacks on my database. Am I reading this right? – RaGe10940 Dec 21 '12 at 17:59
@BillKarwin, I have to say I'm honored to have you expand on one of my posts. How do you ensure that the params get bound with the proper types? – maček Dec 21 '12 at 19:15
@maček, MySQL doesn't care about PHP types for bound parameters, so it doesn't matter what you specify for the PDO param type. The mysqldnd driver handles LONGBLOB differently from scalar types, but PDO doesn't have any support for LONGBLOB anyway. – Bill Karwin Dec 21 '12 at 19:49
@BillKarwin, I'm revisiting this. You need to call `array_flip` on your array of whitelisted params. Otherwise you will end up with an empty result. – maček Apr 04 '14 at 05:54

Understanding Prepared Statements - PHP

2 Answers2