PDO help

ukphp

Hi,

I'm making the move from using MySQLi to PDO and thought I best double check what I'm doing is secure and efficient code? I've got a contact form which accepts user input, then sends the email to a specific email address also writing the email content to a MySQL database. Below is just the connection and insert part of my contact form.

db-config.php

$config = array(
	'host' 		=> 'localhost',
	'username' 	=> 'myUser',
	'password' 	=> 'myPass',
	'dbname'		=> 'myDatabase'
);

contact-form.php

include ('db-config.php');

try {
		$dbh = new PDO('mysql:host='. $config['host'] .';dbname='. $config['dbname'], $config['username'], $config['password']);
	} catch (PDOException $e) {
		echo $e->getMessage();
	}

$sth = $dbh->prepare("INSERT INTO myForm(name, email, email_msg) VALUES (:name, :email, :email_msg)");
$insert = array(
	':name' => $name,
	':email' => $email,
	':email_msg' => $email_msg
);
$sth->execute($insert);

With MySQLi you have to run user input through mysqli_real_escape_string, however am I right in thinking by me using a prepared statement and using :name, :email and :email_msg this is secure? Therefore would I not need to do the following sanitization on user input before inserting into the database?

$name = filter_var($_POST['name'], FILTER_SANITIZE_STRING);

Thanks in advance.

laserlight

Yes, your use of a prepared statement renders the code safe from SQL injection in this case, assuming there are no bugs with the PDO implementation. Note that FILTER_SANITIZE_STRING is not the right approach for escaping data to be inserted into the database in the first place.

ukphp

laserlight;11032195 wrote:
Yes, your use of a prepared statement renders the code safe from SQL injection in this case, assuming there are no bugs with the PDO implementation. Note that FILTER_SANITIZE_STRING is not the right approach for escaping data to be inserted into the database in the first place.

Thank you laserlight! With regard using FILTER_SANITIZE_STRING, do you mean I should be doing the following instead? Out of interest, why should I not be using FILTER_SANITIZE_STRING?

$name = htmlspecialchars($_POST['name']);

bradgrafelman

ukphp;11032199 wrote:
do you mean I should be doing the following instead?

No.

ukphp;11032199 wrote:
Out of interest, why should I not be using FILTER_SANITIZE_STRING?

For the same reason you shouldn't use htmlspecialchars() instead; neither is meant to be used for the purposes of sanitizing data for use in a SQL query. Instead, you should use functions specifically designed to do so, such as PDO::quote() (see [man]pdo.quote[/man]) or, if using MySQLi, [man]mysqli_real_escape_string/man. (Alternatively, using prepared statements with bound parameters also achieves the same result - it's just done internally.)

Bonesnap

ukphp;11032193 wrote:
With MySQLi you have to run user input through mysqli_real_escape_string...

MySQLi supports prepared statements; you do not have to use mysqli_real_escape_string().

bradgrafelman

Bonesnap;11032207 wrote:
MySQLi supports prepared statements; you do not have to use mysqli_real_escape_string().

True, unless you're like me and never bother with prepared statements unless you plan to execute a given query more than once. :p

Bonesnap

bradgrafelman;11032209 wrote:
True, unless you're like me and never bother with prepared statements unless you plan to execute a given query more than once. :p

Forgive my ignorance, but why would executing a given query more than once influence your decision to use prepared statements? Or maybe I am misinterpreting what you mean by a "given query".

bradgrafelman

Bonesnap;11032231 wrote:
why would executing a given query more than once influence your decision to use prepared statements?

shrug Personal preference, I suppose. I just skip the whole 'prepare statement, bind params, execute statement' process and go straight to 'execute raw query' unless I plan to re-use the query with different parameters/values.

At any rate, the OP is indeed using prepared statements; I was simply pointing out the other way of sanitizing data and what function would be appropriate to use in that case. (And note when I say "sanitizing data", I'm of course only referring to protection against SQL injection attacks, not malicious HTML, XSS attacks, etc.)

laserlight

I never use prepared statements when there are no placeholders involved :p

Bonesnap

bradgrafelman;11032235 wrote:
...unless I plan to re-use the query with different parameters/values.

laserlight;11032271 wrote:
I never use prepared statements when there are no placeholders involved :p

Perhaps my understanding of prepared statements is flawed because I've always been under the impression that if values don't change (AKA, hard coded) then using prepared statements is pointless (as is mysqli_real_escape_string()). I don't think anyone uses prepared statements on queries that never change (AKA, hard coded). Or am I just not understanding anything right now? It's noon so it's still pretty early for me.

laserlight

Bonesnap wrote:
Perhaps my understanding of prepared statements is flawed because I've always been under the impression that if values don't change (AKA, hard coded) then using prepared statements is pointless (as is mysqli_real_escape_string()).

bradgrafelman was referring to a case where the values to bind are not hard coded (e.g., they come from user input) but they don't change in the sense that the statement is only executed for that particular set of values in that run of the script. Contrast this to a case where say, you execute the statement multiple times, binding different values each time (where the values might still come from user input).

Bonesnap wrote:
I don't think anyone uses prepared statements on queries that never change (AKA, hard coded).

Probably. There might be some benefit if you happen to be running the same query with hard coded values multiple times within the same run of the script, but I have not encountered such a case where I don't also bind some parameter. Besides, things like a query cache might come into play.