DEV Community

loading...
Cover image for Never Trust User Input

Never Trust User Input

_garybell profile image Gary Bell Originally published at garybell.co.uk ・3 min read

I've had the joys recently of being part of a code audit for a potential client for a change or re-build of a system. The code in itself was complete textbook...of how not to code a system. It looked like it had been built long long ago when OO principles didn't exist, and when no-one knew about security unless they were in that field. Certainly developers knew nothing about security.

It started like any audit I've been involved in, get it set up and then log in. Usually that means asking for a password after checking the database to be sure they aren't stored in plain text. Nope, these were plain text. Ok, so we can get in, and chalk 1 up in the bad developer column. After a small poke around to see what it was doing, it became apparent that every page was its own contained file. There were some includes for navigation, but most of it all stylesheets, JavaScript includes and other common resources were written in each and every file. I wouldn't want to be the one to have to update the version of jQuery on that one!

When we discovered this, we found that the queries being ran on each page were not sanitising any of the user input. They were putting it blindly into queries and using old mysql_* functions; long since deprecated. The bad developer count just hit 2, 3 and 4 from one simple check. There were some instances where the developer was checking for one result being returned and failing if there was more, but that only gets a fraction of a percent off, when I could simply do a 'limit 1' via the GET variable. Still, this was in the application where there users probably aren't technical enough to try, they wouldn't have that sort of code on a login page, right?

I can't even just add 1 to the bad developer for this. It's a minimum of +10 for allowing such simple "hacks" to let someone into the system. Below is an example of literally what I could put as the username:

' or user='admin'#

No password needed. No sanitisation done. I might forgive them if it was a 12 year old system, but the earliest entry in the database (the developer's username) was last year(2015). It used jQuery 1.10 and bootstrap 3.3.8, which only came out in November. Bad person hitting a keyboard (no longer afforded developer status).

Ok, so what can be done to prevent these things? Firstly, stop using mysql_* functions. They were deprecated in PHP 5.5, and there's other alternatives around which are better, like PDO and mysqli_*. That said, in the given example code, I could have done the following with mysqli_query functionality and potentially had worse consequences:

';drop table users;#

Which reminds me so much of the following XKCD comic

XKCD comic referencing database input sanitisation

Aside from using current database interaction functionality, you should NEVER put data sent from a browser into a query. Always sanitise it in one way or another. Ideally use parameterized queries which will take care of sanitisation for you, and also will build the query in a much safer manner (PDO is my friend, it can be yours too).

There's millions of web pages out there on how to prevent SQL injection, how to sanitise data input, how not to code, and why not to trust users. Okay, so there's also a lot of bad resources out there just the same, but there's no reason anyone should be passing anything from the browser straight to the database.
Developers like to blame users when something in the system goes wrong. Sure it would be a user issue if I logged in in that manner, but it's more of a developer issue for allowing it in the first place. Those things aren't limited to just logins either though. If you delete data using the following manner delete.php?id=2 without input sanitisation, I can most likely do delete.php?id=2+or+id+>+0#.

Wave goodbye to your data or, best case, enjoy finding everything being flagged as deleted in the database.

NEVER trust your users to input things correctly, and never trust their input.

Discussion

pic
Editor guide
Collapse
mcastellin profile image
Manuel Castellin

At the end of the day.. "users can't be trusted!"
You make a very good point Gary.. if you want to hear a funny story..

I started my career as a software developer with SQL Injections 🙂

I used to repair printers and computers in a small shop. Business was not great so I had a lot of time to kill.. I thought Kali Linux was very cool thing to learn and (don't tell anyone) I started attacking our client's websites trying to get the site admin password. I managed to hack a few sites and added a "THIS SITE HAS BEEN HACKED" banner in bright red in the homepage 😈

Then I picked up the phone and called each one of them and say, "Hey, I noticed your site has been hacked! By the way, do you know that we also sell CMS and they're all security tested?"

I don't know if that actually lead to any business because I joined a software company a couple of weeks later.. but.. a big thank you to all "developers" that don't sanitize user's data! 🍻 You helped me land my first job as a software engineer!

Collapse
_garybell profile image
Gary Bell Author

That's a great story, made me chuckle.

I've done a few system audits for clients. This particular one we then quoted for a secure rebuild (I worked for a custom software house), but they declined and took our findings to another company. I did one which was for parents to track their kids on their school journey in real time. It was a couple weeks from launch and we discovered such serious issues that we told them if they went live with it, we'd have an obligation to break our NDA and report them to the ICO for a data breach. I think they went out of business a couple months later.

Collapse
mcastellin profile image
Manuel Castellin

Wow, is that for real?! No excuses for taking security lightly with the kid's location! Unbelievable how people just don't care..

Thread Thread
_garybell profile image
Gary Bell Author

They offshored the development to the cheapest bidder. It cost them their entire investment and business. I don't think the business owners realised how bad it was until we showed them.