security

...now browsing by category

 

Sanitize your inputs

Friday, July 17th, 2009

I was hired by a friend of a friend a couple weeks ago to put their new site up that they’d had someone else build for them. I got the impression they used to have someone who could do these sort of release in house but he’d met with an untimely death (or maybe firing, though that’s less dramatic). Regardless, they wanted me to put their new site up for them, which I was happy to do. However when the client put the new version up in a sub-directory of their current siteso I could get access to it, they noticed it wasn’t working like it was supposed to.

At this point they felt like they had a working new site but something was going wrong with the deployment (even though they had actually done the deploying) and it was my job to figure out what was going on. Ok, no problem. I go poking around.

There were a couple issues. First of all, the developer had only used WARNING: SQL INJECTION POSSIBILITY DETECTED

In a flurry of righteous indignation, I email the client and let them know about the problem. I recommend they either confront the developer or, since he obviously doesn’t know what he’s doing, just pay me a little more to give everything a once over. I say, “There’s no sanitation of inputs before stuff is thrown into database queries, which, depending on the sensitivity of the information in your database, could be a big problem. It potentially exposes any information in your database to someone’s prying eyes.”

After a phone call wherein I explain how SQL injection works to the client, they agree to let me fix the problem. A little /^[0-9]+$/ later, we’re back in business. However the client also emailed the original developer and asked if there was any merit to my claims, to which he responds, “while there IS merit to it; it’s never been a problem in any of the stuff i’ve built structured similarly – this includes sites getting hundreds of thousands of visitors a day (for like microsoft, activision, disney/abc, etc). and there’s nothing in the database not viewable on the site itself, so there’s really no real reason to worry about it.”

Now, though a part of me dies inside to say it, let me say his argument isn’t totally without merit. If you only do select statements and if there’s absolutely no sensitive information in the database and if you’re not connecting to the database as a sufficiently privileged user and if you’re using the mysql extension instead of mysqli, I’m not aware of anything particularly malicious you could do with sql injection. But that’s a lot of if statements and we’re talking about something that’s not hard to fix. Just stop being so damn lazy! And of course, just because I’m not aware of any exploits doesn’t mean they’re not out there. One tenet of good programming is to always write your code as if someone smarter than you will try to break it.

Don’t leave holes just because you don’t know how to exploit them. Fill them in and guarantee your security.

mysql_query(“SELECT * FROM table_name WHERE table_id=$_GET[table_id] ORDER BY order ASC”);

– not hard to exploit

if (preg_match(‘/^[0-9]+$/’, $_GET[‘table_id’])) {

mysql_query(“SELECT * FROM table_name WHERE table_id=$_GET[table_id] ORDER BY order ASC”);

}

very hard to exploit

See how easy that was? I usually just make a function isInt so I don’t have preg_match’s all over the place

function isInt($test) {

return preg_match(‘/^[0-9]+$/’, $test);

}

But I needed to prove to that developer that his code was sloppy rather than just taking a “moral” stance so I got to thinking. Did any of those conditions I gave a minute ago jump out at you? “If you’re not connecting to the database as a sufficiently privileged user” but in the code he sent over, the developer was connecting as root so if he was doing that with the demo version of the site he set up on his own server, he should have all the permissions I’d need to make my case. Hmmmm…..

I’m sure there are multiple ways to go about this but I used mysql’s LOAD_FILE command, because I thought that made a pretty strong point.

So, how to exploit SELECT * FROM table_name WHERE table_id=$_GET[table_id] ORDER BY order ASC

Well, a good friend of mine suggested a UNION and that was key (I’d never done this before and never have to use unions). By using a union, you can stick a completely separate query’s result rows onto the initial query’s rows returned and get whatever data you want while still keeping the first query’s column names. The last part there is key because he does a mysql_fetch_assoc and only uses a couple columns.

So just as an experiment, I tried sending in table_id=2 UNION SELECT UNIX_TIMESTAMP() – – which php will then insert as

SELECT * FROM table_name WHERE table_id=2 UNION SELECT UNIX_TIMESTAMP() – – ORDER BY order ASC

The – – is crucial because it tells the server to treat anything afterwards as a comment, basically letting you throw away anything after what you’re inserting (and it’s two separate – marks next to each other but I can’t type that, apparently). You also need the 2 to satisfy there being something for table_id to equal but after that, it’s fair game.

Note, when I say I’m “sending in” table_id=2 UNION SELECT UNIX_TIMESTAMP() – -, what I mean is that I’m going to the site’s url with that parameter. Though this isn’t the site’s actual url, it’s like doing this:

http://baddeveloper.com/myclient/actions.php?table_id=2 UNION SELECT UNIX_TIMESTAMP() – –

Anyway, of course, that query doesn’t work as an experienced unioner could’ve told me because unions require both select statements to have an equal number of columns. And I could tell the sql was invalid because I stopped getting any output from php because it was generating an error in the mysql_query. So I started throwing in commas until BAM, I started getting output again. I ended up with table_id=2 UNION SELECT UNIX_TIMESTAMP(), 2, 2, 2 —

Now at this point I could tell the query was valid because the script was showing results again but it wasn’t showing my timestamp because the script only showed certain columns’ results and apparently the first one wasn’t one of them. I ended up with the timestamp in the third spot and the final query before I got really dirty was table_id=2 UNION SELECT 2, 2, UNIX_TIMESTAMP(), 2 – -. Now the script would dump out my timestamp but just being able to do that wouldn’t prove this was a real, dangerous exploit.

mysql’s LOAD_FILE is a nasty command. It lets any user with sufficient database privileges load the full data from a file on the system into a query result. For example, SELECT LOAD_FILE(‘/etc/whatever’) will return the contents of that file in the result. I assume there are restrictions about which files you can access based on the user mysql is running as but that didn’t prove to be an issue. Once I changed my query to table_id=2 UNION SELECT 2, 2, LOAD_FILE(‘/etc/passwd’), 2 – -, I was getting a list of all the users on the system and their home directories.

At this point, I could’ve gotten pretty malicious. I just used sql injection to find a huge hole in this guy’s system and could’ve stolen massive amounts of data, but maliciousness wasn’t really the point. I emailed him with a link to the exploit and admonished him with, “That’s why you always sanitize your inputs. I fixed it on our end but you should probably do it on yours too. Even if you’re just doing SELECT’s, there are vulnerabilities when you don’t validate.” And knowing is half the battle.

He has yet to respond or fix the hole in his own site. Nevertheless, it was a glorious victory and I think I provided a lot of value to my client by catching a gaping issue in their site and hopefully disuading them from using an inexperienced developer in the future.

Life Achievement Earned: Actually Hacked Something

Validate your inputs. Even if you don’t think you have to.