How security flaws work: SQL injection

This easily avoidable mistake continues to put our finances at risk.

Read the whole story

 

[joshv]

Good summary of the problem. I thought I knew everything about SQL injection already, but 'blind' injection was a new one for me, and just highlights how any hole, no matter how small, can be exploited when you can automate the attack.

 

[lthown]

Hey, no discussion of SQL injection is complete without referencing "Exploits of a Mom"

 

[psd]

Likewise, tutorials and training need to do start doing things the right way from the outset.

A massive effort should be launched to downvote or outright remove answers that suggest string concatenation of SQL statements from places like StackOverflow and other forums.

 

[Maldoror]

Thirty-one-year-old Laurie Love is currently staring down the possibility of 99 years in prison.

Am I the only one who thinks this is a disproportionately long custodial sentence for this crime? Surely a few years in the slammer would set him straight.

 

[ChrisSD]

It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly but at this point there is little excuse for such elementary problems.

Personally I've found the slogan "parametrize, not sanitize" helps inexperienced devs to remember good practice. The only issue is they sometimes take it a bit too literally but I can't figure out a pithy catchphrase that explains sanity checks have their place so long as it's built on top of prepared statements and not as a replacement to it.

 

[bthylafh]

Maldoror[/url]":g9hqizrl]

Thirty-one-year-old Laurie Love is currently staring down the possibility of 99 years in prison.

Am I the only one who thinks this is a disproportionately long custodial sentence for this crime? Surely a few years in the slammer would set him straight.

'Murrica.

 

[April King]

Seeing md5($password) in the article's primary image was enough to give me serious heartburn. I certainly hope that screenshot is of code that is 20 years old.

 

[grape tomato]

This is a great article, thank you. As a newbie to this kind of stuff who has been building some internal tools for my job, it's eye-opening to see what people with bad intentions are able to do. Our next "try to break it" dev session is going to include some of these attacks, even though the stakes are much lower (if you've already gotten through enough security to be on our internal network, we're screwed regardless).

 

[Peevester]

Mostly on point. One note about the classic "1=1" thing. Using it to change results with AND is definitely done, but OR is even more useful. Say you wanted to dump the whole table instead of just one record - you could set your query parameter to "1234 OR 1=1", and now instead of just getting 1234, the query returns the whole table. In places where the page outputs the results in tabular form, this can give you much more data than you could easily find poking around for values. It's even more useful if someone is using PHP, or concocting the query in a page's javascript (I know! It still happens though).

In terms of mitigation, really all you need to know is the last section - DO NOT make queries out of strings from user input, ever. That applies both to values in predicates, and in the query parts themselves. I can remember seeing a form that someone wrote where the user could select ordering, and the form returned the name of a variable, ostensibly from a list (to be stuck at the end of an ORDERED BY), but of course you could submit something completely different with wget or whatever. Prepare your queries, bind your host variables. Period.

One thing I've never understood, because it didn't really come about in database APIs until the web, is submitting multiple queries at once. A lot of SQL injection techniques (including the "little Bobby tables" method of using " '; " and "--" to neuter the first query) would be impossible without being able to do it, and a well-written app shouldn't NEED to do it - just submit two dang queries like your IQ was normal. If you're going to do any sanitizing of SQL statements (don't, see above), at least dump everything after the first semicolon.

 

[Rommel102]

The ultimate conclusion for the escalating hacking wars is AI vs AI. First, the defensive companies will implement it, then the attackers. It will ultimately come down to the brute force of one AI versus another.

Any software that isn't designed with security foremost in mind and that isn't able to quickly update after new exploits are discovered will be a weapon.

 

[psd]

quietnine[/url]":2zo9kne0]Proper escaping folks. That's all we ask.

Proper API's to interact with the database is the better thing to ask for.

 

[fellow human]

Wow SQL string building, I remember that. Thanks for the walk down memory lane

Even from an effort perspective it's hard to imagine choosing to do that over using something like an ORM. Won't someone think of the migrations!

 

[Dayvid]

ChrisSD[/url]":zhixxq4h]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly but at this point there is little excuse for such elementary problems.

Personally I've found the slogan "parametrize, not sanitize" helps inexperienced devs to remember good practice. The only issue is they sometimes take it a bit too literally but I can't figure out a pithy catchphrase that explains sanity checks have their place so long as it's built on top of prepared statements and not as a replacement to it.

It's partially because prepared statements aren't a complete solution and don't allow for some use cases - eg anything involving structural statements. You can use a prepared statement to make SELECT A,B FROM TABLE WHERE C = ?param safe, but as an arbitrary example SELECT A, ?field FROM TABLE is doomed - there's no way I can specify that as a particular field from that database table in a prepared statement. There's workarounds (eg using a giant case/if else/whatever statement to build the query from comparing ?field to preapproved values if they're static) but they're ugly enough that people can avoid them.

You can argue don't do that and for a lot of cases that's reasonable, but it's common enough that this still happens. As soon as you're in that situation you're back with the trying to sanitize inputs and it doesn't really matter how careful you are, there's probably sql injection bugs.

 

[fellow human]

Dayvid[/url]":3g8zxmao]as an arbitrary example SELECT A, ?field FROM TABLE is doomed - there's no way I can specify that as a particular field from that database table in a prepared statement.

Pull * (or a subset) and only return the requested field. More sent over the wire but it's safe and optimization-friendly.

 

[rockforbrains]

ChrisSD[/url]":2pdtmwof]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly but at this point there is little excuse for such elementary problems.

Personally I've found the slogan "parametrize, not sanitize" helps inexperienced devs to remember good practice. The only issue is they sometimes take it a bit too literally but I can't figure out a pithy catchphrase that explains sanity checks have their place so long as it's built on top of prepared statements and not as a replacement to it.

I think there are a couple of reasons for this. First most devs/dbas are fluent either in programming languages or SQL but rarely both. Second many common languages do not have good native facilities available to parameterize the input. The frameworks that have excellent facilities require one more item to study. So, when the PHB says build a website that does whatever by x date you are often stuck with using the tools and skills you already have.

I my personal situation I am programmer with a decent knowledge of SQL and very limited knowledge of the frameworks and parameterizing APIs. So if a PHB says build a website quickly I would naturally fall back on what I know. Though I am aware of SQL injection and know it is reasonably straightforward to block with the right tools I am not very skilled at those tools (and obviously I know it). If I have the time I would learn one the tools but often the PHB does not want to delay a project because they have made promises that must be kept or they lose face with upper mismanagement.

 

[cbreak]

ChrisSD[/url]":1rxhp40h]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly but at this point there is little excuse for such elementary problems.

Personally I've found the slogan "parametrize, not sanitize" helps inexperienced devs to remember good practice. The only issue is they sometimes take it a bit too literally but I can't figure out a pithy catchphrase that explains sanity checks have their place so long as it's built on top of prepared statements and not as a replacement to it.

I think there are a couple of reasons for this. First most devs/dbas are fluent either in programming languages or SQL but rarely both. Second many common languages do not have good native facilities available to parameterize the input. The frameworks that have excellent facilities require one more item to study. So, when the PHB says build a website that does whatever by x date you are often stuck with using the tools and skills you already have.

I my personal situation I am programmer with a decent knowledge of SQL and very limited knowledge of the frameworks and parameterizing APIs. So if a PHB says build a website quickly I would naturally fall back on what I know. Though I am aware of SQL injection and know it is reasonably straightforward to block with the right tools I am not very skilled at those tools (and obviously I know it). If I have the time I would learn one the tools but often the PHB does not want to delay a project because they have made promises that must be kept or they lose face with upper mismanagement.

That sounds like an idiotic excuse... "Sorry, we couldn't test the batteries enough because we had to release before the new iPhone came out"... Don't sacrifice quality and actually working for releasing faster. Sacrifice features instead.

 

[Dayvid]

rockforbrains[/url]":2viv61xp]

ChrisSD[/url]":2viv61xp]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly but at this point there is little excuse for such elementary problems.

Personally I've found the slogan "parametrize, not sanitize" helps inexperienced devs to remember good practice. The only issue is they sometimes take it a bit too literally but I can't figure out a pithy catchphrase that explains sanity checks have their place so long as it's built on top of prepared statements and not as a replacement to it.

I think there are a couple of reasons for this. First most devs/dbas are fluent either in programming languages or SQL but rarely both. Second many common languages do not have good native facilities available to parameterize the input. The frameworks that have excellent facilities require one more item to study. So, when the PHB says build a website that does whatever by x date you are often stuck with using the tools and skills you already have.

I my personal situation I am programmer with a decent knowledge of SQL and very limited knowledge of the frameworks and parameterizing APIs. So if a PHB says build a website quickly I would naturally fall back on what I know. Though I am aware of SQL injection and know it is reasonably straightforward to block with the right tools I am not very skilled at those tools (and obviously I know it). If I have the time I would learn one the tools but often the PHB does not want to delay a project because they have made promises that must be kept or they lose face with upper mismanagement.

You really, really need to be aware of prepared statements and how to use them if you're creating a website. They're not complex at all and are the only reliable way of dealing with multiple families of security bugs.

 

[ChrisSD]

rockforbrains[/url]":1zh2hw6a]

ChrisSD[/url]":1zh2hw6a]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly but at this point there is little excuse for such elementary problems.

Personally I've found the slogan "parametrize, not sanitize" helps inexperienced devs to remember good practice. The only issue is they sometimes take it a bit too literally but I can't figure out a pithy catchphrase that explains sanity checks have their place so long as it's built on top of prepared statements and not as a replacement to it.

I think there are a couple of reasons for this. First most devs/dbas are fluent either in programming languages or SQL but rarely both. Second many common languages do not have good native facilities available to parameterize the input. The frameworks that have excellent facilities require one more item to study. So, when the PHB says build a website that does whatever by x date you are often stuck with using the tools and skills you already have.

I my personal situation I am programmer with a decent knowledge of SQL and very limited knowledge of the frameworks and parameterizing APIs. So if a PHB says build a website quickly I would naturally fall back on what I know. Though I am aware of SQL injection and know it is reasonably straightforward to block with the right tools I am not very skilled at those tools (and obviously I know it). If I have the time I would learn one the tools but often the PHB does not want to delay a project because they have made promises that must be kept or they lose face with upper mismanagement.

I can understand that an amateur (in terms of web development) may not know what they don't know. This doesn't excuse professional web devs nor the companies that not only won't hire professionals but also won't give their current employees the time to get some basic knowledge of the problem domain.

Btw, what common languages support sql but not prepared statements? I'm struggling to find one.

 

[maxwell]

April King[/url]":34e0evw0]Seeing md5($password) in the article's primary image was enough to give me serious heartburn. I certainly hope that screenshot is of code that is 20 years old.

phpBB came out in June 2000 so you are pretty close....

 

[jdguzman]

It's interesting that the author didn't mention tools like ActiveRecord. Granted when used incorrectly there is still the possibility of leaving yourself open to injection, but I still think it's a viable option to prepared statements.

 

[ramases]

Dayvid[/url]":4qulgo2b]It's partially because prepared statements aren't a complete solution and don't allow for some use cases - eg anything involving structural statements. You can use a prepared statement to make SELECT A,B FROM TABLE WHERE C = ?param safe, but as an arbitrary example SELECT A, ?field FROM TABLE is doomed - there's no way I can specify that as a particular field from that database table in a prepared statement. There's workarounds (eg using a giant case/if else/whatever statement to build the query from comparing ?field to preapproved values if they're static) but they're ugly enough that people can avoid them.

You can argue don't do that and for a lot of cases that's reasonable, but it's common enough that this still happens. As soon as you're in that situation you're back with the trying to sanitize inputs and it doesn't really matter how careful you are, there's probably sql injection bugs.

First, IME true need for that type of dynamic query generation is much more rare than commonly thought off. Rare enough that I would refuse any such change during review, unless compelling need can be demonstrated.

Besides, many of the widely reported cases of successful SQL injection did not fall under it. There was absolutely no excuse for not using prepared statements there.

Second, you can safely generate such queries using prepared statements and without if/else monsters: Every single API worth using provides for a way to dynamically retrieve the column names for a table; there is no need for hardcoded lists.
All you need to do is to check each requested field name against this whitelist, and if a match is found, use the column name from the whitelist instead from the user-supplied input; if no match (or more than one match ) is found, you log a security incident.

Once you've sanitized your column names this way, you use that to generate the prepared statement, and bind parameters.

That's something you stick into some utility part of your code-base if you truly need this capability, and I honestly have troubles thinking of a scenario where there is both a legitimate need for this type of query generation but where column-name-comparison against the table structure would be too much effort.

 

[Dayvid]

ramases[/url]":je20vx62]

Dayvid[/url]":je20vx62]It's partially because prepared statements aren't a complete solution and don't allow for some use cases - eg anything involving structural statements. You can use a prepared statement to make SELECT A,B FROM TABLE WHERE C = ?param safe, but as an arbitrary example SELECT A, ?field FROM TABLE is doomed - there's no way I can specify that as a particular field from that database table in a prepared statement. There's workarounds (eg using a giant case/if else/whatever statement to build the query from comparing ?field to preapproved values if they're static) but they're ugly enough that people can avoid them.

You can argue don't do that and for a lot of cases that's reasonable, but it's common enough that this still happens. As soon as you're in that situation you're back with the trying to sanitize inputs and it doesn't really matter how careful you are, there's probably sql injection bugs.

First, IME true need for that type of dynamic query generation is much more rare than commonly thought off. Rare enough that I would refuse any such change during review, unless compelling need can be demonstrated.

Besides, many of the widely reported cases of successful SQL injection did not fall under it. There was absolutely no excuse for not using prepared statements there.

Second, you can safely generate such queries using prepared statements and without if/else monsters: Every single API worth using provides for a way to retrieve the column names for a table.
All you need to do is to check each requested field name against this whitelist, and if a match is found, use the column name from the whitelist instead from the user-supplied input; if no match (or more than one match) is found, you log a security incident.

This way, even if someone managed to trick your comparison function into accepting malicious input, the "damage" is limited that they can perhaps choose a different field, a capability they'd implicitly have in this scenario anyway.

Once you've sanitized your column names this way, you use that to generate the prepared statement, and then proceed as usual.

That's something you stick into some utility part of your code-base if you truly need this capability, and I honestly have troubles thinking of a scenario where there is both a legitimate need for this type of query generation but where column-name-comparison against the table structure would be too much effort.

Agree these cases should be exceptional and avoided wherever practical and this is the case for almost all "normal" websites. Sometimes it's not without compromising something else. The solution you're mentioning here is what I'm talking about with the case/if else.

 

[psiclone]

SQL injections are super easy to prevent and the agencies and businesses that fail to protect against them should be the ones held accountable more than the hackers. Period. I don't condone hacking, but if they find something like this (basically the equivalent of having an unlocked door to your house...people see it and assume they can't get in, but a thief would try and easily open it), the defending party is irresponsible and negligent.

 

[Tofystedeth]

But with developers apparently unwilling to do things the right way, it's time for the creators of these frameworks to do the right thing and force all database access to go through prepared statements. This won't make it impossible for wayward developers to use string manipulation and continue to create highly exploitable flaws,

Ain't that the truth.
I once inherited a (thankfully, internal only) web application that used prepared statements, except then didn't bother to actually use real parameterized inputs for most of the user supplied values. That was fun to clean up.

 

[photochemsyn]

ChrisSD[/url]":3dkln2ot]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly . . .

It might be related to the lack of consequences for sloppy work in the field. I.e. Let's say in contrast you design a chemical production system with equally bad flaws. The consequences of sloppy work are explosions, toxic leaks, people dying, massive damages and lawsuits. Whereas in web development work, leaky flawed systems look good and seem to work fine, and since the startup owner doesn't want to pay for comprehensive testing of the system, just wants to start generating revenue as soon as possible. . .

 

[psiclone]

photochemsyn[/url]":lcqyodev]

ChrisSD[/url]":lcqyodev]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly . . .

It might be related to the lack of consequences for sloppy work in the field. I.e. Let's say in contrast you design a chemical production system with equally bad flaws. The consequences of sloppy work are explosions, toxic leaks, people dying, massive damages and lawsuits. Whereas in web development work, leaky flawed systems look good and seem to work fine, and since the startup owner doesn't want to pay for comprehensive testing of the system, just wants to start generating revenue as soon as possible. . .

Also, keep in mind that a lot of web developers never got any real formal schooling. Many are self-taught and pick up things as they go, so they'll likely get their training off of poorly designed online tutorials or courses done by individuals who might not know or care.

There's also the other end of things, whereby the developer might try to go down the right route, but is told to go another way because it's cheaper, faster, and "unnecessary."

 

[fenncruz]

Thirty-one-year-old Laurie Love is currently staring down the possibility of 99 years in prison.

Am I the only one who thinks this is a disproportionately long custodial sentence for this crime? Surely a few years in the slammer would set him straight.

Its probably the maximum, which is no where near what you actual get

 

[Ronin007]

Dayvid[/url]":8nehhpdr]

ChrisSD[/url]":8nehhpdr]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly but at this point there is little excuse for such elementary problems.

Personally I've found the slogan "parametrize, not sanitize" helps inexperienced devs to remember good practice. The only issue is they sometimes take it a bit too literally but I can't figure out a pithy catchphrase that explains sanity checks have their place so long as it's built on top of prepared statements and not as a replacement to it.

It's partially because prepared statements aren't a complete solution and don't allow for some use cases - eg anything involving structural statements. You can use a prepared statement to make SELECT A,B FROM TABLE WHERE C = ?param safe, but as an arbitrary example SELECT A, ?field FROM TABLE is doomed - there's no way I can specify that as a particular field from that database table in a prepared statement. There's workarounds (eg using a giant case/if else/whatever statement to build the query from comparing ?field to preapproved values if they're static) but they're ugly enough that people can avoid them.

ALL SQL is a string, submitted to the database. Doesn't matter the source - prepared statement, stored procedure, etc.

Dynamic SQL - where the query is constructed entirely on the fly based on user criteria - is a reality in any database to accommodate customization for things like reporting. There's no getting away from implementing the logic to construct the string/query; all you can do is use API provided by the vendor. The Curse and Blessings of Dynamic SQL is a highly recommended read for those whose product uses Microsoft SQL Server.

 

[Ronin007]

photochemsyn[/url]":14uehz1j]

ChrisSD[/url]":14uehz1j]It is baffling to me that there are web devs writing for some major sites who are writing code in such a flawed way. I don't want to call them incompetent exactly . . .

It might be related to the lack of consequences for sloppy work in the field. I.e. Let's say in contrast you design a chemical production system with equally bad flaws. The consequences of sloppy work are explosions, toxic leaks, people dying, massive damages and lawsuits. Whereas in web development work, leaky flawed systems look good and seem to work fine, and since the startup owner doesn't want to pay for comprehensive testing of the system, just wants to start generating revenue as soon as possible. . .

There are safeguards for such industries where software issues will put lives at risk, which is typically reflected in the cost as satisifying those requirements costs time and money.

As for non-critical development, my experience is that the industry supports sloppy work because most do not know what their market is when development starts. The number of clients I've dealt with who didn't really conceptualize their software until they had to version 1.0... So local shops started providing basically interactive screenshots, which clients took to be a working prototype so the clients balked at delivery times... Not getting too heavily invested is just good business sense.

 

[Ronin007]

ramases[/url]":1p2focg3]
Besides, many of the widely reported cases of successful SQL injection did not fall under it. There was absolutely no excuse for not using prepared statements there.

Second, you can safely generate such queries using prepared statements and without if/else monsters: Every single API worth using provides for a way to dynamically retrieve the column names for a table; there is no need for hardcoded lists.
All you need to do is to check each requested field name against this whitelist, and if a match is found, use the column name from the whitelist instead from the user-supplied input; if no match (or more than one match ) is found, you log a security incident.

Once you've sanitized your column names this way, you use that to generate the prepared statement, and bind parameters.

That's something you stick into some utility part of your code-base if you truly need this capability, and I honestly have troubles thinking of a scenario where there is both a legitimate need for this type of query generation but where column-name-comparison against the table structure would be too much effort.

You've never had to deal with a poorly performing query before. Native SQL is the only means - ORM has never been the silver bullet, and prepared statements still require someone to have actual SQL knowledge.

For a small, trivial application what you advocate works. But not for larger scale implementations that deal with non-trivial relationships. The crux of the article, issue and your comment is that ignorance is not to be condoned by abstracting something that someone never learn in the first place.

 

[abj]

 

[millad]

For any PHP programmers out there reading this and are thinking "Okay, but what do I do about it?" Please use and implement PDO (PHP Data Objects). Read through the manual on php.net and search for "(The only proper) PDO tutorial - PHP Delusions", and of course "PHP The Right Way"

Obviously, you can use a framework to help you if you like, but it is not required at all and you could still shoot yourself in the foot with a framework. Understand how to use PDO. All of your database efforts going forward will be much easier and safer for everyone.

 

[psiclone]

millad[/url]":2uvrwmjo]For any PHP programmers out there reading this and are thinking "Okay, but what do I do about it?" Please use and implement PDO (PHP Data Objects). Read through the manual on php.net and search for "(The only proper) PDO tutorial - PHP Delusions", and of course "PHP The Right Way"

Obviously, you can use a framework to help you if you like, but it is not required at all and you could still shoot yourself in the foot with a framework. Understand how to use PDO. All of your database efforts going forward will be much easier and safer for everyone.

The trick is when you have over 100 fields to deal with. That gets really fun...NOT!

 

[Ronin007]

psd[/url]":1kky9atc]

Likewise, tutorials and training need to do start doing things the right way from the outset.

A massive effort should be launched to downvote or outright remove answers that suggest string concatenation of SQL statements from places like StackOverflow and other forums.

So you [and your upvoters] do not know what dynamic SQL is, or when to use it...

Allowing everyone to vote undermines the value of an answer because there's always a signal-to-noise ratio issue for cases like yours.

 

[NoMorePosting]

Back in the days of SQL Server 6, 6.5, 7 and 2000 when I did a lot of database development, SQL injection was a known risk. Even then there were easy tools to prevent it. Access only by Stored Procedures, pararmeterized queries, read only table access permissions (for when it had to be an adhoc query), triggers on tables detecting row quantity (and rolling back)

I'm sure those tools are expanded now - that injection is still a thing, says more about the developers, than the product. Constructing all the SQL text in the client is rarely required, but is just easy.

 

[millad]

psiclone[/url]":kmp9vvoc]

millad[/url]":kmp9vvoc]For any PHP programmers out there reading this and are thinking "Okay, but what do I do about it?" Please use and implement PDO (PHP Data Objects). Read through the manual on php.net and search for "(The only proper) PDO tutorial - PHP Delusions", and of course "PHP The Right Way"

Obviously, you can use a framework to help you if you like, but it is not required at all and you could still shoot yourself in the foot with a framework. Understand how to use PDO. All of your database efforts going forward will be much easier and safer for everyone.

The trick is when you have over 100 fields to deal with. That gets really fun...NOT!

What part is not fun? It certainly isn't difficult, if that's what you mean? Having 100's or more fields in a table in a DB is inconsequential to the topic. If you are programming in PHP and don't have any idea what PDO is, then AT A MINIMUM, it is worth your time to read and understand.... even if you use an ORM or framework.