Question on converting existing site from mysql_* to PDO

[maxxd]

Hey y'all. Hopefully quick question on something  I've not come across before.

 

I am doing a quick and dirty update on an existing site that is using the mysql_* functions to use PDO, and I'm wondering how much of a corollary there is between the mysq_real_escape_string() function and PDO::quote() method. We had a sanitization method that returned the submitted string after running mysql_real_escape_string() on it, and I've updated it to return the string after passing it through quote(). What I'm noticing in phpMyAdmin, though, is that new records inserted using the quote() sanitize don't encode quotes or add slashes or evidence any of the things that apparently mysql_real_escape_string() used to do (I always used a different scrub method in the past so I'm really not familiar with how it works under the hood).

 

Is using quote() going to offer an equivalent level of protection against injection?

 

Hopefully I'll get the go-ahead to take the time and revamp all the queries to use prepared statements, but right now that's not in the cards. At least the old site did abstract database interaction so I'm not chasing mysql_* functions all over the site...

 

Any opinions and thoughts are very much welcome and thanks in advance!

[ginerjm]

I do believe you will hear from many here that using pdo's quote() is unnecessary.  With PDO you have access to prepared queries which are the preferred way of running queries this century.  Read up on them and use them whenever you have to include user-supplied input as part of your query.

[fastsol]

It would be dumb not to convert to prepared statements, if you're already doing the work to convert to PDO and change the code, then you might as well change it all at once.  I converted my entire CMS in only a few hours and I had dozens of things that needed to be changed.  It actually wasn't that difficult nor that time consuming.  Adding the prepared statements later in your case is literally almost double the work as apposed to doing it right the first time.

[maxxd]

I agree it would be better to do all at once, unfortunately that decision isn't up to me. Hopefully we'll get to it soon, but right now not so much. We've got in-line queries scattered all throughout the site that are already using the existing sanitization method. Obviously I don't want to just allow the user to insert unsanitized data directly into the queries, and I was hoping we could get by usig quote() until we can convince the client to let us redo the site entirely. At that point, it's prepared statements and not a worry for the future...

 

I do use PDO and prepared statements, but this was a legacy piece that was inherited. The transition actually came about because I was tasked to handle some very minor maintenance on another part of the site and noticed the msql_* functions in the abstraction class. Brought it to the attention of the higher-ups and they were kind enough to listen and let me run with it despite the crazy backlog of work we're looking at.

[kicken]

What I'm noticing in phpMyAdmin, though, is that new records inserted using the quote() sanitize don't encode quotes or add slashes or evidence any of the things that apparently mysql_real_escape_string() used to do

Even with mysql_real_escape_string there should not be any kind of visible escaping after your data has entered the database. If you are seeing things like backslashes before quotes within the database when querying with phpMyAdmin then you're actually double-escaping the data which is incorrect.

 

Using quote() as a replacement for mysql_real_escape_string is fine, however as suggested you should move to prepared statements as soon as you're able to. Note that unlike mysql_real_escape_string the quote method will add quotes around the string so you'll need to either update your queries to not include quotes or strip the leading and ending quotes.

[ginerjm]

With the amount of re-write you will have to do to switch over to pdo, how can you NOT alter the queries at the same time?  You will simply remove the existing values and replace them with the placeholders and then modify the existing 'query()' call to 'execute(array(key=>value,key=>value))'.  Of course you have to reference the un-sanitized values but that too should be simple.

[maxxd]

Even with mysql_real_escape_string there should not be any kind of visible escaping after your data has entered the database. If you are seeing things like backslashes before quotes within the database when querying with phpMyAdmin then you're actually double-escaping the data which is incorrect.

 

Using quote() as a replacement for mysql_real_escape_string is fine, however as suggested you should move to prepared statements as soon as you're able to. Note that unlike mysql_real_escape_string the quote method will add quotes around the string so you'll need to either update your queries to not include quotes or strip the leading and ending quotes.

Yeah, I found out about quote() wrapping the string already and took care of that. And honestly, I didn't think I should see the escaping in the database - I'm wondering exactly what was done in the past that resulted in the issue to begin with. When doing my own code, I always used htmlentities() or htmlspecialchars(), so I was a bit confused. Thanks for the info!

 

With the amount of re-write you will have to do to switch over to pdo, how can you NOT alter the queries at the same time?  You will simply remove the existing values and replace them with the placeholders and then modify the existing 'query()' call to 'execute(array(key=>value,key=>value))'.  Of course you have to reference the un-sanitized values but that too should be simple.

I completely agree and I look forward to getting in there to do exactly that.

 

Thanks for the info and advice, everybody - I'm going to mark this thread complete but feel free to keep the discussion going...

[ginerjm]

HTH with my $.02

[kicken]

And honestly, I didn't think I should see the escaping in the database - I'm wondering exactly what was done in the past that resulted in the issue to begin with.

The typical cause was running with magic_quotes_gpc enabled which would automatically apply addslashes to all input data and then escaping the data again before putting it into the query without first undoing the magic quotes effect.

[maxxd]

The typical cause was running with magic_quotes_gpc enabled which would automatically apply addslashes to all input data and then escaping the data again before putting it into the query without first undoing the magic quotes effect.

I am so afraid that's the culprit. The server is run by the client, and unless they specifically turned on magic quotes, I'm a little concerned that php may have not been updated in however long. At which point I can just revert all the changes and let everything run on mysql_*, but still...

[Jacques1]

First of all, PDO::quote() in the context of a MySQL database actually is mysql_real_escape_string(). They both call the exact same MySQL library function. The only difference is that PDO::quote() also wraps the result in single quotes.

 

Much has been said about prepared statements already, and it's great that you're planning to use them in the future. However, many people aren't aware why they're worth the extra effort. Why not just escape the values?

 

The problem is that escaping is highly susceptible to errors. There are not only the obvious mistakes like forgetting to escape a value, forgetting the quotes, making wrong assumptions about “safe” values etc. You can also run into much nastier issues. Try out the following code, for example:

<?php

/*
CREATE TABLE
	users
(
	name VARCHAR(255) PRIMARY KEY,
	password VARCHAR(255) NOT NULL
)
CHARACTER SET big5
;

INSERT INTO
	users
SET
	name = 'admin',
	password = 'hunter2'
;

*/



$db = new PDO('mysql:host=localhost;dbname=test', 'YOUR_USER', 'YOUR_PASSWORD');

// never use SET NAMES to change the character encoding!
$db->query('SET NAMES big5');

// user input
$_POST['name'] = 'admin';
$_POST['password'] = "\xbf\x27 OR 1-- ";

$user_stmt = $db->query('
	SELECT
		name
	FROM
		users
	WHERE
		name = ' . $db->quote($_POST['name']) . '
		AND password = ' . $db->quote($_POST['password']) . '
');
$user = $user_stmt->fetchColumn();

var_dump($user);
 

We clearly escape the input with PDO::quote(), so an SQL injection should be absolutely impossible. Yet this is exactly what happens: The user is able to log-in as the admin by performing an SQL injection through the password parameter.

 

The problem is that escaping depends on the character encoding of the database connection. In order to find critical characters like the single quote, MySQL must know how they're actually represented on byte-level. The encoding information is set to the default encoding (usually latin1) when you connect to the database, and it's updated when you change the encoding with the charset parameter of the PDO constructor. However, what what we do above is use a SET NAMES query. This also changes the character encoding of the connection, but it does not update the encoding information. So PDO::quote() still thinks you're using latin1 when you've actually switched to big5. This mismatch makes the function blind and breaks it entirely.

 

Fortunately, many character encodings used today (ASCII, ISO 8859-1, UTF-8 etc.) are more or less compatible with each other, so you usually get away with encoding bugs. But I wouldn't rely on this. In any case, the example shows how fragile escaping is.

 

Note that what PDO calls “prepared statement” is by default just automated escaping and has all the issues outlined above. If you want actual prepared statements, you have to explicitly set the PDO::ATTR_EMULATE_PREPARES attribute to false.

[maxxd]

Jacques1 - great info - thanks!