Hacker News Reader

Hacker News

Comment by thomasfl | Hacker News Reader

Full thread

thomasfl

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

View on HN

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

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

Reply on news.ycombinator.com