Security hole in Facebook Footprints example

As I was working on Funhouse Photo last night, I ran into a bug where a database insertion operation was failing. I realized that this was because I had copied the Footprints example, and created the query using string concatenation, eg:

$res = mysql_query('SELECT `from`, `to`, `time` FROM footprints WHERE `to`=' . $user . ' ORDER BY `time` DESC', $conn);

In my case, it was an internally generated string that was causing the problem, because it contained an unescaped single quote. I realized that I was also passing in strings I'd received from the user via POST. This means that someone could easily run an SQL injection attack, and get full access to my database!

An injection attack works by passing a user input string that contains a quote and semi-colon to prematurely finish the query command the host thinks they're running. The rest of the string contains some other arbitrary command that the attacker wants to run. In the query above, if $user was actually passed as

12345'; DROP TABLE footprints; --

then the complete query would read

SELECT `from`, `to`, `time` FROM footprints WHERE `to`='12345'; DROP TABLE footprints; --'ORDER BY `time` DESC

In this example a table's deleted, but you could insert almost any SQL statement.

(Edit - PatMan pointed out in the comments that mysql actually has a nice security feature that only allows a single statement in a PHP mysql execute. This is reassuring, it definitely makes it harder to do any damage, though it still leaves the door open to plenty of data-access exploits)

There are two main ways to avoid this. In PHP4 and earlier, magic quotes is turned on by default, which means all posted strings have their quotes auto-magically prefixed with backslashes. This turned out to be a dodgy solution for a lot of reasons, and it was turned off by default in PHP5, which is what I'm running.

A better fix, and the one I ended up using, is to call mysql_real_escape_string() on any strings before you concatenate them into a query. You could also use something other than concatenation to create your query.

Now, I'm a PHP/mysql newbie, but it seems like a pretty Bad Thing that Facebook's example app contains this security hole. Newbies like me are also the ones least likely to already know that it's a security problem, and so will copy it blindly into their own apps.

Hopefully the guys over at Facebook will be able to update the example, before it infects too many other apps, and gives hackers access to all the personal data that's being stored. I have posted on the developer discussion board, but it's unclear if there's any other way to alert the team.

Funhouse Photo User Count : 5. I worked on the 'send a photo to a friend' feature last night, that's still not working (a story for another post) so I've held off publicizing it. It is in the queue for the directory now though.

Comments

[PatMan]

dude,

Doing "12345'; DROP TABLE footprints; --" will do absolutely nothing to your database. PHP/MySQL only executes the first query it encounters.

Try it you will see

[Pete Warden]

Thanks PatMan! I didn't know that about that feature of PHP's mysql interface, my testing was through the command line. I've updated the article.

That is definitely a handy security feature, and limits the damage an attacker can do. I'm still planning on escaping my user input though, since otherwise I'm still open to plenty of data access exploits.