top | item 2383857

How not to protect against SQL injection (view source)

249 points| ssclafani | 15 years ago |cadw.wales.gov.uk | reply

115 comments

order
[+] somedev|15 years ago|reply
It was me that actually built this site. Around 2000-2001. To give you a bit of background or "excuses":

It was my first website at an agency, I'd just taught myself ASP and SQL in just a few months previous (with no help or guidance). If my memory serves me correct, that dodgy JavaScript was put in there by a more senior developer. I had no idea what SQL Injection was and it wasn't until at least a few years later that SQL Injection was even something any developers I knew were aware of - The Wikipedia page for SQL Injection (http://en.wikipedia.org/wiki/SQL_injection) under "Known real-world examples" has the earliest dated at 2005 (but obviously, this vulnerability has been around forever).

And yes, I'm still a Web Developer (front-end nowadays - that also knows much better than this) and no, I no longer work for that agency and haven't for a long time.

In response to some of the comments: * I've seen many many developers write SQL Injection prone code at least 6 years after this was written. * Any developer that was around during 2000-2001 would know that this was before the time of CMS's (free or otherwise), libraries, frameworks, SQL abstraction layers etc. * I'm pretty sure there is some server-side sanitising done too (before we'd heard of the term SQL Injection). * I don't think it was using an SQL login with drop permissions.

[+] mixmax|15 years ago|reply
I just fired off an e-mail to point out that they have a potentially serious security problem and they should get it fixed ASAP.

I see this as a civic duty, and think that this is the kind of action you're required to perform if you see a serious problem. Writing an e-mail takes ten seconds, but the potential damage could well cost serious money.

[+] gaius|15 years ago|reply
This is the government we're talking about... Whoever wrote this won't get sacked for gross negligence, which is what this is, as much as say, operating machinery without a safety cover, and is probably paid waaaay more than anyone with comparable "skills" in the private sector.
[+] derleth|15 years ago|reply
If you're lucky, you aren't in the UK so they won't be able to arrest you instantly on the hacking charges.

If you're very lucky, the place you are in won't honor their demands for extradition on the hacking charges.

[+] csomar|15 years ago|reply
I once hacked into a Twitter Mass following tool. It was written with the .net stack and not obfuscated, so I was able to get the whole project source code.

I sent an email to the stuff out there. I typed "I just hacked your software. It's insecure and not obfuscated. Tell the devs".

The guy replied accusing me of hacking the software and told me that he is going to sue me. I just replied "F*ck you and your team. I was just playing around with no intent to make any damage for your company; but now I'll release a cracked version and distribute it".

I hadn't released a cracked version, though.

[+] Joeri|15 years ago|reply
The saddest part is that tons of people will be reading this thinking that they're way smarter than that guy, while in fact their sites are wide open to exploitation as well. That last statement probably applies to me too.

Doing web security well is hard, too hard. Everyone gets caught with a security bug sooner or later, even google. It's easy to laugh with silly coding like this, but I blame the technology for allowing SQL injection in the first place. SQL is simply a bad API to be using in a web app.

[+] Skalman|15 years ago|reply
I don't think SQL is a bad API - it's just that every language makes it so difficult to use prepared statements! It shouldn't be harder than:

    sql_query('SELECT * FROM mytable WHERE name = ?', name)
(I'm aware that this defeats the purpose of prepared statements to be reusable - this is just an API that's better than the current methods)
[+] quattrofan|15 years ago|reply
Web security is hard, but I don't think its unreasonable to expect that someone you hire understands the basics of how to make a form submission secure, if they don't perhaps they should be flipping burgers instead. There is just no excuse for something so sloppy.

As for SQL being a "bad API" that might be one of the more ridiculous comments I have heard.

[+] mncolinlee|15 years ago|reply
This is a great point. As engineers, it's a great lesson that we need to stop apologizing for our tools and instead contribute to an effort to make them better. The natural language basis of SQL is very handy, but it does not lend itself well to detainting of input. There should be strict segregation until a late phase between what the developer wrote and what the site user wrote.

There is no excuse why programming languages should require the programmer to work around the inherent deficiencies of SQL. Just like the x86-64 architecture introduced the no-execute bit to the mainstream computing world and made the rampant buffer overflow remote code execution exploits of the 1990s into an endangered species, we should look to a technological solution to solve SQL's deficiencies.

Why should we need to trust that thousands of CMS module developers all use, know, and understand detainting of input in their native language? If we want to allow reuse of code, we put our absolute trust into thousands of developers other than ourselves.

[+] wladimir|15 years ago|reply
It's not like this is an obscure corner-case that the developer forgot. In that case I'd agree with you.

This is a principal issue. Doing client-side parameter validation for security is a stupid idea in every case. Client-side validation is for user convenience only. This has nothing to do with SQL. Security must be implemented server-side. There are no exceptions to this.

[+] somedev|15 years ago|reply
See my comment above :)
[+] elboru|15 years ago|reply
"Terms & Conditions: SECURITY We are committed to ensuring that your information is secure. In order to prevent unauthorised access or disclosure we have put in place suitable physical, electronic and managerial procedures to safeguard and secure the information we collect on line. We use encryption when collecting or transferring sensitive data such as credit card information."

I don't know why but I just don't trust them...

[+] ZeroComplete|15 years ago|reply
I'm going to assume that they have a server-side validation script running and the client side code is just to prevent/explain to mistaken users and if the server-side script every activates they know that someone's being malicious.
[+] julianc|15 years ago|reply
Maybe the javascript is intentional, like a honeypot for hackers :)
[+] spicyj|15 years ago|reply
Right, I tried searching for "select" and it just redirected me back to the home page.
[+] donniefitz2|15 years ago|reply
I was thinking the same thing. Let's hope that's true.
[+] MatthewPhillips|15 years ago|reply
Since they're using SQL Server (hint is that they are checking for "xp_"), you can get a list of all of their databases with "SELECT name FROM sys.databases", then loop through and drop them. Hope the web login doesn't have drop permissions.
[+] fleitz|15 years ago|reply
Heh. One of the best protections against SQL injection is not to grant those privileges to the db account from the webserver. So many sites could protect themselves at the least from data loss by using the built in tools. Also, I've worked on projects where the only way to change a table is via a stored proc, no select access on the tables, no SQL injection. Or at least you'll have to figure out a way to to the injection into the stored proc which is nigh impossible unless they are using exec_sql in the store proc.
[+] tptacek|15 years ago|reply
Are they actually vulnerable? How do you know? People have gotten in serious trouble in the UK for "innocuously" testing web apps for SQL problems. Know that in both the UK and the US, you are taking a significant risk by prodding websites like this.
[+] MatthewPhillips|15 years ago|reply
If destruction isn't your fetish, you could also get all of the table names from the databases, and use sp_send_dbmail to send the SELECTs for all of them to your email address. There might be an easier way to get the data out though.
[+] RossDM|15 years ago|reply
When I was working in the financial sector, I came across an email thread involving a certain software vendor who had been notified of a SQL injection vulnerability. To fix it, they created an IF statement that did a string comparison to check for the exact SQL attack that had been used.
[+] Animus7|15 years ago|reply
I have a feeling that this site won't be up much longer after making front page of HN, and it will have nothing to do with server load.
[+] goalieca|15 years ago|reply
It was on reddit yesterday I believe which means tons of kiddies saw that. People were guessing that it included sanity checking on both sides because it wasnt down yet.
[+] _b8r0|15 years ago|reply
H.M. Government has a specific set of standards that apply to websites based on the impact of information assets contained on them (as well as other bits and pieces that I don't need to go into). The weird thing is, this site is for the Welsh Assembly which, as a devolved government has to meet the standards but is seen in certain respects as a 'foreign government' within the civil service (our H.M. Government sector). Make no mistake, there are some things that this site will have to comply with, but the implied and genuinely air-quoted 'measures' put forward would add nothing to any of this.

A moderately large amount of this information is available on the Internet, start at http://www.cabinetoffice.gov.uk/resource-library/security-po... if you want a look. A brief look through the sitemap suggests they are holding or processing Personally Identifiable Information (PII) which puts them under the Data Protection Act. Again, the presence of the javascript doesn't imply actual SQL injection, but it definitely doesn't imply a measure against it.

In this instance, the compliance requirements are fairly low. I guess the exam question is, can they pass the bar, or do they limbo under it?

[+] jofabian|15 years ago|reply
Funny is that I tried to warn them about that problem and their Feedback form doesn't work.
[+] dayjah|15 years ago|reply
facepalm - though the 1/4 Englishman in me snickers at the Welsh!
[+] Stormbringer|15 years ago|reply
Would have been better protected if their javascript was programmed in Welsh... :D
[+] arpy|15 years ago|reply
Poor old Bobby Drop Tables will be out of luck again.
[+] teichman|15 years ago|reply
So for those of use who know nothing about websites: what is the correct way to protect against SQL injection?
[+] tajddin|15 years ago|reply
Generally, on the server-side, you parameterize the query. Depending on the server-side language, a normal SQL query that would read SELECT * FROM myTable WHERE lastName = 'Smith' would be converted to something like SELECT * FROM myTable WHERE lastName = @lastnameparam. Then in code, you'd supply the value of @lastnameparam as 'Smith'.

It depends on the language, but this is what you'd do in .NET, for example. In this case, the framework does the work for you by encoding the value of lastnameparam (it makes sure that whatever is supplied to lastnameparam isn't read as SQL).

[+] pyre|15 years ago|reply
Well, since it wasn't necessarily answered explicitly anywhere, you don't want your validation code to run on the client (i.e. the browser) because the end-user has absolute control and can easily circumvent your controls. You have to protect yourself on the server-side where you have control.
[+] stevemoy|15 years ago|reply
My take on this is that the scriptwriter's goal was not to stop SQL injection attacks but rather prevent regular users from inadvertently screwing with the database.

Looking at it that way makes it a much more understandable (and all-too-common, unfortunately) oversight.

[+] MattJ100|15 years ago|reply
Erm... if the server-side was already escaping properly then there would be no way for users to mess with the database. Only if it is not escaping properly is this code vaguely useful.

It's not like you can't store semi-colons in an SQL database :)

[+] gary4gar|15 years ago|reply
Javascript - It can be Disabled!

Every Web Dev needs to remember this and Yet people tend to forget

[+] dasil003|15 years ago|reply
A better principle is that you can not trust the client. Not only can javascript be disabled, it can also be modified, a new web page can be created posting to this endpoint, or one can send raw HTTP requests without a browser at all.
[+] MatthewPhillips|15 years ago|reply
They use javascript to submit the form, so that's not a vulnerability for them.
[+] d2|15 years ago|reply
There are more websites than competent admins so this kind of thing is inevitable. If you were a nice guy you would have reported it to the admin and left it at that.
[+] rosenjon|15 years ago|reply
Yeah...pretty sad. But at the same time, if the site isn't down by now, there is probably server side checking in place as well.
[+] Lozzer|15 years ago|reply
The linked page seems very safe. It has a very bad form checking function, but no actual form...
[+] JohnnyBrown|15 years ago|reply
Well, it's still up after 8 hours, so apparently there was some server-side checking as well