How not to protect against SQL injection (view source) | Hacker News Reader

How not to protect against SQL injection (view source)

cadw.wales.gov.uk

249 pointsssclafani115 comments

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.

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.

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.

No offense, but I think you're a tad paranoid. If I was a mechanic and I saw someone at a gasstation driving a car that was obviously dangerous because of some kind of bad fixup I would tell him. This is no different, and I don't expect anyone to sue me for that.

Here's the mail I sent:

Hi there,

It appears that you have some pretty severe security problems on your site. This is a heads up so you can get it fixed. I would recommend doing so ASAP.

Your site has been posted to hacker news (which is a friendly programming site for start-up people and nerds) as an example of bad security practices. The link is here: http://news.ycombinator.com/item?id=2383857

It has also been posted to Reddit, which might be more of a problem since that site has a lot of 14 year old bored teens hanging around that know just enough about programming to do a lot of damage... Link: http://www.reddit.com/r/programming/comments/gdviz/how_not_t...

It appears that your site is easy to compromise, which might lead to anything from defacement to someone stealing all your content, usernames, passwords, etc.

I have nothing to do with these postings, I just don't like to see innocent sites get in trouble, hence this mail. Feel free to contact me if you need anything or have questions.

Hope you get it fixed before someone breaks it.

Yours,

Max

I've got $10 riding on "they see the word Hacker in 'Hacker News' and start freaking out". :D

Oh, I understand the word hacker in all its culturally and context relevant forms, and you understand the word hacker, but they do not understand the word hacker. :-(

That's why we have to tell them.

If we, as hackers of the sort that inhabit hacker news, have a post like this on the frontpage and noone cares to actually write them and tell them they have a security problem that may cause them serious damage we don't deserve better.

Perhaps an anonymous email would have been safer.

Edit: But your email is very amiable and clear, so I think you're probably safe.

I think he deftly smoothed that potential problem over with the statement in parenthesis, as well as mentioning how it was posted in reddit.

You're right, getting sued for pointing this out would be absurd. But where you're wrong, is that just might've happened. There are more ridiculous lawsuits out there. There was a story on here a couple of years ago where a US Government real estate web site was using JavaScript for their authentication, with the password stored in plain text. When it was pointed out to them via email, they responded with allegations of computer fraud and threatened to file charges. Obviously they weren't 100% serious and I'm sure nothing official came of it - but yes, people this stupid are that ridiculous.

I sent a email when i saw it too.

Actually got a thank you email back yesterday, them seemed pleased :-) See what they did there america? You don't have to sue everyone.

I wouldn't say they were paranoid. A few months back I showed a colleague what looked like the openings of a very serious data leak in a major company's site. He investigated further and then reported it up through the chain of command and then over to the company. At no point did he do anything other than what was done here, as in point out a publicly visible security flaw. He was nearly fired after the company threatened to sue. The company only relented when he agreed to keep quiet and his employers disciplined him. The employers didn't back him up. All this for solely reporting a flaw, absolutely zero use of said flaw.

I lost a lot of my faith in humanity that day.

Is that the typical response to bug reports, or an exception to the rule?

I would go find a new employer if that was the case, what bunch of idiots.

Getting penalized - sometimes heavily - for pointing out someone else's security problems can and does happen:

http://thedailywtf.com/Comments/Hack-School.aspx

The Computer Misuse Act 1990 makes no provision for intent (this is partially provided through the Police and Justice Act 2005 amendments). Instead, the act talks about unauthorised access, which is an undefined term.

In fact, a security consultant was convicted[1] for using ../../ in a URL after he thought a site had been hacked.

[1] - http://www.out-law.com/page-6207

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.

Actually, speaking as a Brit working in IT who has supplied various public sector organisations in the past, I think it's rather more likely that this would have been written by an outside agency to contract. And I wouldn't expect they were paid waaaay more from that experience either.

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.

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.

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)

it's just that every language makes it so difficult to use prepared statements

Huh? Most web frameworks use ORMs and discourage you from touching SQL at all.

Which is great until you discover that your needs can't be fulfilled by the ORM and you need/have to use SQL.

ie, in every non-trivial or legacy-supporting use. Such as everything in existence for a non-brand-new company.

A good ORM such as SQLAlchemy can do (nearly) everything that SQL can, it even knows quite some specific options for SQL dialects.

Yes, there might be cases that a very specifically optimized query needs SQL, but those should be the exception not the rule.

The side benefit of prepared statements, perhaps even more importantly if security is not really your concern, is that you don't need a password page that looks like this (this is really the password requirements page for my school):

A password must:

    be 6-8 characters in length.
    contain a non-alphanumeric character such as ( ! ] & * , + =

A password cannot:

... include a dollar sign ( $ ), a single quote ( ‘ ), a double quote ( “ ), a number sign ( # ), a less-than sign ( < ), a question mark ( ? ), a pipe ( | ), a back quote ( ` ), or a backslash ( \ ). ...

I hate those signups. The worst part about them is that they're usually on sites I'm required to sign up for, like a school, work, or corporate service.

If it was a startup web app I was signing up for, I'd send the developer a polite email saying that I didn't feel comfortable putting my data in such a system. Unfortunately, all I can usually do is gripe a little in private.

Fun fact: Etrade won't let you use a period ( . ) in a password. No error or anything, account creation just silently fails.

You don't need one of those anyway, if you hash your passwords like you should.

This is the second time in a few days someone has made the point that web stacks make it hard to use prepared statements†. This is a one-liner in Rails. Does it not work in Python? How hard is it in PHP?

† Which are not a cure-all for SQLI.

I think the problem angle is slightly different: it's too easy to use simple string concatenation for SQL. That works across pretty much every web stack, so it's the path of least resistance to get something working, and many a developer never bothers to learn the proper idiom anew for every framework.

Side note: We're jumping to conclusions by thinking that the javascript is the entire implementation. It's perfectly possible that the server is already safe against SQL injection and the javascript is just an extra line of defense. Maybe the client and server were done by separate programmers and the client programmer wanted to make sure he wouldn't get blamed. It's a government website: nobody in government ever got fired for being too careful. Or maybe the programmer had to do it to satisfy some non-technical bureaucrat who wanted to think that hacking attempts couldn't even reach his server.

I wrote fairly recently about SQL Injection in Python at http://www.simple-talk.com/sql/learn-sql-server/sql-injectio... .

I do not address web stacks directly there, but Python itself will happily let you use prepared statements, it is up to the programmer to take effective steps to prevent SQL Injection.

Very easy in PHP. http://php.net/manual/en/pdo.prepared-statements.php

Except when the database drivers PDO relies on aren't set up by default:

http://news.ycombinator.com/item?id=2376873

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.

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.

About this case specifically, you're right. This person wrote some dumb code. However...

It is indeed unreasonable to expect a junior developer to make form submission secure. Aside from SQL injection there's DoS, MiTM, CSRF, XSS, session fixation, cache poisoning, clickjacking, timing attacks (to detect valid vs invalid values), rainbow table attacks, and many more. Just go through the list of requirements in OWASP ASVS. It's intimidating how much stuff there is to keep track of. We have a dedicated security engineer on our team who reviews all new code, out of necessity.

Also, about SQL being a bad API, I didn't say it was a bad API in general, just bad for the web. SQL is like eval(), it evaluates code from a parsed string. If eval() is bad for the web, SQL is just as bad.

If indeed it was a junior developer then where was the supervision? I take your point on SQL though.

See my comment above :)

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.

"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...

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.

I was thinking the same thing. Let's hope that's true.

It would seem kind of stupid if they were smart enough to implement validation but not smart enough to limit user access to it. Of course there's no accounting for the depths of stupidity.

That would actually not surprise me at all. There are a lot of Web devs who can make a site that renders in the browser and mostly works, but can't wrap their minds around the difference between server-side code and client-side. Browse through the JavaScript tag on Stack Overflow and you'll come across more than you can shake a stick at. Many people (either due to willful ignorance or a sad gap in their education) write functions like:

    function getTime() {
      <?php return time(); ?>
    }

It's not at all improbable that somebody told them their site was vulnerable to SQL injection, so they took a brief glance at the Wikipedia page and said, "I know, I'll just stop people from writing this stuff." So they open up the page where people might be entering the malicious text and write some code that will stop them. They run it in their browser and it works — none of their SQL strings make it through. They have now fixed the problem, as far as they are concerned, and the site owner doesn't know enough to tell them how utterly braindead their approach is.

There may actually be a good reason for writing code like that. The time the content was generated by the server, perhaps?

That wouldn't be a good reason for writing that code. Putting it in a function like that suggests that you expect the value to change. It's like the xkcd joke where a random() function is implemented to return 4, as determined by a fair dice roll. If you just wanted to store the time the page was generated, it would make more sense to use a constant — for example:

  window.pageBirthday = <?php echo time(); ?>

Also, if they're on Stack Overflow asking why it doesn't correctly report the current time, that's a pretty good indicator that they're simply mistaken.

Yeah, that function is useful for calculating the difference between server time and client time.

Right, I tried searching for "select" and it just redirected me back to the home page.

http://www.reddit.com/r/programming/comments/gdviz/how_not_t...

Based on some of the comments there, it doesn't look like it (or at least, it wasn't there several hours ago when Reddit stumbled upon the site). See http://www.reddit.com/r/programming/comments/gdviz/how_not_t... for some examples.

Maybe the javascript is intentional, like a honeypot for hackers :)

Yeah, maybe the 100+ validation errors in the markup are like a honeypot for web designers too ... I've not seen 1x1 gifs for a few years now.

And leaking the MS SQL server errors and IIS errors are just adverts for MS (I only did genuine searches, "hotel" got me to an error page).

I'm sure the silly long names are part of the ruse too.

There is much that could be done with this site. Perhaps I could drop them a CV.

See my reply above. Yep, 1x1 gifs and table layouts were cutting edge back then ;)

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.

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.

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.

I have no idea if they are, hence "I hope their web login doesn't have drop permissions".

Since people on Reddit are apparently actually poking this thing, I just want to get in the warning: DON'T DO THAT.

Yea, dropping tables or otherwise destroying their server is a very bad idea. FYI: http://news.ycombinator.com/item?id=2358058

That's what those 5-euros-a-month VPN services are for, I assume.

"Testing" a web app when it is not your job to do so is self evidently a suspicious act, just like walking down a line of cars testing the door handles to see if any have been left unlocked would be a suspicious act.

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.

If you take away write-access privileges, then malicious script still can steal your data. Most of the time - that's really bad by itself even if you keep your copy of data intact.

Why the downvotes? It is common sense to not run software with more permissions than it needs. Your production app shouldn't connect to your database through the root account, for example, unless it needs full, complete permissions to function. Not likely.

Of course, this does not eliminate the need for solid code-based prevention of injections...

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.

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.

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.

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?

Funny is that I tried to warn them about that problem and their Feedback form doesn't work.

facepalm - though the 1/4 Englishman in me snickers at the Welsh!

Would have been better protected if their javascript was programmed in Welsh... :D

Poor old Bobby Drop Tables will be out of luck again.

So for those of use who know nothing about websites: what is the correct way to protect against SQL injection?

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).

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.

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.

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 :)

Javascript - It can be Disabled!

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

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.

They use javascript to submit the form, so that's not a vulnerability for them.

You can submit their form without using their JS

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.

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.

The linked page seems very safe. It has a very bad form checking function, but no actual form...

Well, it's still up after 8 hours, so apparently there was some server-side checking as well

Well at least form & function are equals.

Hello!!! Has anybody hacked the site yet? Perhaps it would be nasty to delete all database tables but at least some sort of update would be funny?

https://addons.mozilla.org/en-us/firefox/addon/groundspeed/

They are not scared of sql injection cause they have Styled Scrollbars!!

http://news.ycombinator.com/item?id=2370022 (CEO Friday: Why we don’t hire .NET programmers)

Would an open source programmer do something like this?

cough php cough

>Would an open source programmer do something like this? Why not? http://www.eweek.com/c/a/Security/Oracles-Suncom-Hit-Along-w...

Upvote here if you too have discovered sql injection vulnerabilities in your own web apps.

How about no.

If you vote no, you haven't looked hard enough.

No, really - I'm certain there aren't any in my code. SQL injections are extremely easy to protect against if you know how it works. There might however be other vulnerabilities.

You trust the library doing that protection for you too?

Yeah? What i your "extremely easy" mechanism for avoiding SQLI? Is it, as I surmise from your previous comment, "using prepared statements for everything"? Because that isn't bulletproof.

It's bulletproof if you don't use string concatenation in your prepared statements.

EDIT: No this doesn't limit you to 'simple queries'! How do you figure that? There are only a VERY small subset of problems you can't solve like this. So small that in 10 years I've only had to do it once and I write SQL Server 5 hours a day.

Want to give me an example please?

So, in other words, it's bulletproof if you only use simple queries.

In MySQL, for instance, LIMIT and OFFSET have to be integer constants; the wire protocol won't allow you to bind variables to them. Does your SQL engine allow you to parameterize a table name? Can you parameterize columns? What about ASC and DESC? And this is just simple stuff. What about pages with "Advanced Search" that have to implement query builders?

I've never used MySQL (and God willing I'll never have to) but in SQL Server you can limit rows by setting @@ROWCOUNT before your SELECT statement. SQL Server also allows CASE in your ORDER BY clause that can do pretty much anything you could want.

I've never had a situation where the client enters the column names to return in the UI. I mean the users should not have to know the column names in your database so surely you'll do that some other way instead? It's pretty rare (and probably wrong) to have hundreds of columns being returned, so we'd just return them all and show/hide the relevant ones on the application-side.

Same for table names. Why would you need to have a parameterized table name? This has never come up in all my years of SQL Server. Sounds like bad DB design or something exotic that I've never had to do. I mean how would you index queries like that anyway?

For your 'Advanced Search' I'd probably use a temp table or table variable and do the query in multiple steps using 'IF' switches depending on the flags or setting passed to it.

All of those scenarios can be handled in such a way that you're only concatenating known strings rather than user input - e.g.

stmt = "SELECT col1 FROM table ORDER BY col2 " + (isDescendingSort ? "DESC" : "ASC")

As long as all your user input has been filtered through type checking, enumerations, etc. (aside from parameters), is that not a safe approach?

Of course you can build these queries safely.

Of course you should use prepared statements when possible.

But web devs do have a bad habit of saying "we're safe, we used prepared statements", and then losing their app within 5 minutes because of the code than handles sortable columns in their table views.

I don't do webapps, but I'm sure everyone's vulnerable somewhere, if they do.

I just dislike the whole "upvote for x" comment that dredges up from Reddit.

'Just lowers the signal-to-noise ratio, and I hate to see things like that creep up on HN.

I wrote an iPhone app with a sqlite backend that was vulnerable to SQLi. Don't think you aren't vulnerable even if your application doesn't touch the internet.

Sorry, I just don't do any apps that touch the internet...just some programming for fun on the side.

The only thing that I've written that could be applied to this is our POS system at the restaurant I work at as a dishwasher and cleaner for. It's in Django though, and the Django project takes care of most issues with that...not that they're really priority #1 security-wise...

You should never assume that your framework of choice does everything for you. This is by all means no shot at Django, but just in general, always assume what you are working with is insecure and full of bugs - and then account for that - if your framework/programming language of choice accounts for additional things - great.

But does this mean, for example, that you should escape inputs yourself before passing them off to the framework, which is then ostensibly going to escape them again?

I think a better approach is to verify that the framework is correct. You can do this experimentally, by writing unit tests, or by reading and running the unit tests of the framework itself.

If you assume the framework is correct, and then you update, migrate, whatnot, can you still be sure the framework is correct, or hasn't broken. If you can ensure your own code is good, then you are ahead of the game in such a situation.

I don't complain about down votes - but it actually shocks me to think someone felt that my statement was counterintuitive to this thread and didn't offer anything possibly insightful. I think that it is irresponsible to assume third party code is safe - or will remain safe. If you feel that that is overly cautious so be it - but I rather be safe than sorry. But I guess that is just my opinion.

If you use type-safe SQL parameters everywhere and no string concatenation you'll be fine.

http://taylorza.blogspot.com/2009/04/sql-injection-are-param...

"Upvote here" doesn't work on HN.

P.S. Learn how to set up a poll, if you want something similar.

Depends on your definition of 'work' doesn't it?

Just trying to help somebody get used to the community norms.

Reply on news.ycombinator.com