Uncle Sam's had it up to here with 'unforgivable' SQL injection flaws

The US has clearly had enough of software vendors shipping products with "unforgivable" vulnerabilities, and is now urging them to launch formal code reviews to stamp out SQL injection flaws. The Federal Bureau of Investigation (FBI) and Cybersecurity and Infrastructure Security Agency (CISA) issued a Secure by Design Alert on …

[Mike 137]

"developers allow user-input data to be supplied to a database directly as a SQL command"

This is one of the technical reasons why the adversary rarely needs "sophisticated attacks". For every genuinely targeted incident there are (probably) thousands that result from merely being wide open as the attacker passes. Do you leave your front door unlocked or put the key under that plant pot on the porch? No? So why do the equivalent with your public online interface? My impression is that there are two key reasons -- naivety and penny pinching. Regardless of what the devs do, the client could have the finished work exhaustively tested before going live, but they don't. I remember an international group company the CIO of which said to me "but we don't have enemies", and a major online portal the commissioning director of which refused to pay for it to be pen tested. These are not exceptions. In 20+ years of consulting I've found it to be pretty much the rule.

[bombastic bob]

Re: "developers allow user-input data to be supplied to a database directly as a SQL command"

One method I try to use (always) is to make sure that the data itself will never contain characters that might cause SQL injection, such as a single quote mark or semicolon., Or I can explicitly escape them somehow before building the SQL command myself. This gives you the opportunity to test your code to see how it responds to a data field like "Boo'; DROP TABLE MASTER;" and so you check for the single quote, the semicolon, and so on, and test for control chars that might also do something bad. Then you can create generic SQL good for any DBMS that's also safe, use utilities like "psql" to do the dirty work, and so on.

But yeah getting that sanitized input takes some effort. Not hard with PHP though. Time worth spending regardless.

[OhForF']

Re: "developers allow user-input data to be supplied to a database directly as a SQL command"

Even if you do some validation on the input to stop sql injection you should be using parameterized statements and not build your sql statement as a string containing user supplied values.

I.e. instead of doing something like:

$result = $conn->exec('INSERT INTO Students(firstname, lastname, email) VALUES (' . $firstname . ', ' . $lastname . ', ' .$email .')' );

you use

$stmt = $conn->prepare("INSERT INTO Students (firstname, lastname, email) VALUES (?, ?, ?)");

$stmt->bind_param("sss", $firstname, $lastname, $email);

$stmt->execute();

In the last version it should not be a problem if you have to enroll Little Bobby Tables.

[ldo]

Re: you should be using parameterized statements

The article itself says that is not a recommended solution.

[sedregj]

Re: you should be using parameterized statements

The article says:

"Software vendors have been advised to use parameterized queries with prepared statements to mitigate SQL injection vulnerabilities."

So what is the difference between a parameterized query and a statement as given in the example here?

[ldo]

Re: you should be using parameterized statements

The article also says:

These are deployed by some vendors, but were branded "brittle" by CISA and the FBI. They said they're also difficult to deploy on a large scale and are more easily bypassed.

[SVD_NL]

Re: you should be using parameterized statements

That statement refers to input sanitation, not parameterized statements.

[Michael Wojcik]

Re: you should be using parameterized statements

Yes. To be fair, the antecedent of "These" in that sentence was not immediately clear; I had to take a second look myself.

[Richard Pennington 1]

Re: you should be using parameterized statements

Input sanitation is for when you know you're feeding it crap.

Perhaps it's time to flush the memory.

[Greybearded old scrote]

Re: "developers allow user-input data to be supplied to a database directly as a SQL command"

You will never be able to list all the bad things to filter.out. Go the other route, with an allow list, and you will almost certainly not include every legitimate input.

If it was easy we'd be paid less.

[yetanotheraoc]

Re: "developers allow user-input data to be supplied to a database directly as a SQL command"

"If it was easy we'd be paid less."

Don't worry, the next dev will be paid less.

Using parameterized statements gets around malicious data at time of insert. But then you have these malicious data lurking, ticking time bombs waiting for a casual select with unexpected side effects. So you need all of input scrubbing and parameterized inserts and separate accounts for update vs read. Sql injection is the same as all vulnerabilities, it needs defence in depth and the willingness to always ALWAYS do the right thing. But there's no budget for that.

[ldo]

Re: You will never be able to list all the bad things to filter.out.

No need to “filter out” anything, just make sure to properly quote your strings. For standard SQL, this is as easy as

q = "'" + s.replace("'", "''") + "'"

[CowHorseFrog]

Re: "developers allow user-input data to be supplied to a database directly as a SQL command"

the truth is 9 / 10 developers shouldnt be allowed to progream anything.

[Headley_Grange]

" hold their vendors to account by asking them if a formal code review into a product's susceptibility to SQL injection exploits has occurred and what mitigations have been put in place. "

That's assuming a lot of things. Buyers might not even know that the product they are buying has an SQL database in it, never mind how security is coded, reviewed and tested. When I buy a phone I don't question the vendor about how they did the EMC testing. When I buy a toaster I don't ask the bloke in Curry's about how they did the electrical safety testing. I don't have to - there's a legal requirement, a certification process and they're not allowed to sell products that haven't been certified. Code exploit vulnerabilities - across the board, not just SQL - have the potential to hurt people and society and should be treated the same as electrical safety.

[sitta_europea]

"... they're not allowed to sell products that haven't been certified."

How I wish that were true. But it isn't. You can buy unsafe electrical products from Amazon any day of the week. I've been writing to Trading Standards and my MPs (yes, plural) for YEARS about this and they've all done sweet FA -- even after I've sent them press cuttings about people dying as a result of their apathy.

An SQL injection attack *probably* isn't going to kill anybody directly, although I wouldn't want to say it couldn't, but -- while they'll do nothing about 13A plugs which cause fires because they have no fuses in them, cables which catch fire because the conductors aren't copper but copper plated iron, and forged CE markings on more or less anything -- I don't expect anybody to get excited enough about coding to amend the legislation.

[Yorick Hunt]

"forged CE markings"

But those CE markings are real - in this case though, "CE" stands for "Chinese Electronics."

[A.P. Veening]

But those CE markings are real - in this case though, "CE" stands for "Chinese Electronics."

Worse, they stand for "China Export" as they aren't allowed in China itself.

[Anonymous Coward]

"How I wish that were true. But it isn't."

No, it is true - they aren't (legally) allowed to sell them, but that doesn't physically prevent them (much in the same way as you're not allowed to drive at 100mph on the motorway).

That the MPs are apathetic, and Trading Standards don't have the budget to enforce it is another problem.

[Anonymous Coward]

Can't see it happening

A pop singer once wrote "every generation throws a hero up the pop charts"

he could have been referring to every generation knows better than the last and has to roll it's own code. Again and again.

Imagine if bricklayers had to design a new trowel for every job ?

[Glenn Amspaugh]

Lil' Bobby Tables-

fugitive on the run.

[Anonymous Coward]

We live in a capitalist society.

Code reviews cost resource which means it costs money. Does it increase profits? Nope. What are the consequences for not doing it? Not much. Maybe a little reputational harm and having to do a patch to fix it.

Does the cost to fix equal more than the code review? Nope. There is the answer as to why they won't do it when they don't have to.

As for sanitizing inputs and parameterized queries. That's the first thing I learn when playing about in or learning a new language as and when they are relevant. Why wouldn't you?

[An_Old_Dog]

Does the cost to fix equal more than the code review? Nope.

What is your basis for writing this? The accepted technical wisdom (vs accepted managerial wisdom) (/me puts on fake graybeard and porkpie hat) is that the earlier in the development process an error is caught, the less it costs to fix it. It's cheapest to catch it in the specification creation stage. It's less-cheap to catch it in the program design stage. It's even-less-cheap to catch it in the code review stage, it's even-even-less-cheap to catch it during the code testing/debugging stage, and it's even-even-even-less-cheap to catch it via end-user bug report after release.

Managers frequently tell themselves, and others, "It'll be fine. We don't need to waste time and money on code reviews."

Of course, some companies don't even "waste" time and money on making fixes, even after major problems have been reported. This is because, as you've pointed out, there's no effective penalty mechanism.

[ChoHag]

So what you're saying is, it's cheapest to not fix it at all?

[Anonymous Coward]

It's cheapest not to have the problem in the first place.

Do you really need code reviews if the people writing the code truly know what they're doing? It's not like real developers default to insecure SQL and only do it correctly when reviewed and caught, then go back to being crap for the next project.

OK, it costs more to hire competent developers in the first place, but it saves the cost of problems created by crap ones.

[Michael Wojcik]

Do you really need code reviews if the people writing the code truly know what they're doing?

If you have to ask, you're not a "real developer".

[disgruntled yank]

a capitalist society

At this point it is a bit hard to find any other kind. The PRC claims not to be, though. Do you think that the US National Security Agency finds that Chinese shops write more secure code?

[stephajn]

Coders vs Developers

This is one of those things that needs to be pounded home for everyone who is "learning to code". String building SQL commands etc etc should just be an absolute no no. It should be right up there with storing credentials in plain text form.

This is such low hanging fruit to just eradicate these unforgivable practices from everywhere. I know I've had to maintain some legacy applications in my career where whoever originally built it did these things, and I raised it up the flagpole and was shot down with the same old garbage of "Well it works, and we aren't going to pay for the work needed to fix it." And that's exactly why these no-brainer data leaks are possible.

But good luck convincing the bean counters of the need to invest in this stuff.

But this is the difference between someone who "learned to code" vs someone who actually DEVELOPS something.

[Anonymous Coward]

Re: Coders vs Developers

How about "if the tutorial you are following requires that you install an obsolete version of PHP, because the supported versions literally do not support that insecure API, try a different tutorial..."?

How does SQL injection work with modern code? I haven't come across anything newer than PHP4/5 examples that doesn't use parameterized queries... And those PHP4/5 examples won't run on a modern server, because they removed the API. Newer languages/frameworks have had the correct APIs since the beginning.

[doublelayer]

Re: Coders vs Developers

"How does SQL injection work with modern code?"

Nothing will ever stop you from submitting just one string. They don't parse to see if you're passing something that looks like a value and insist you parameterize it. This means that, if someone either doesn't know or doesn't think to parameterize, they can still build an expression as a string and that expression is still vulnerable. Most likely, you're just not seeing as much of it because the number of databases out there is massive and a lot of programmers were told not to build expressions that way and remembered it. Sadly, that doesn't mean everyone was taught that, so it still happens year after year.

[Scene it all]

Re: Coders vs Developers

Perhaps the database vendors can take some responsibility here and just not expose any "execute this string" functionality in their APIs. We all got used to programming without GOTOs.

[bombastic bob]

Re: Coders vs Developers

Ahem. count the number of 'goto' statements in the Linux and BSD kernel source. Efficient code often requires old-school methodology.

[doublelayer]

Re: Coders vs Developers

Effectively, this requires them to completely change their interfaces in a way that makes them less flexible. Libraries that make it easy for you to parameterize a query are still turning it into a string when they send it to the engine, they're just doing it better than you would on your own. There are three reasons why database engines won't make that change:

1. Writing parsers becomes much harder. Consider all the possible parameters to a statement as simple as select. You can sort in a variety of ways, you can select from multiple things and combine them, you can use one statement to filter them. Now write a function contract in C that can do all the same things. What parameters do you need to take if the query might reference multiple tables. What parameters do you need to take to expose all of the internal functions of the database and construct a function that can be used to sort them? Your function is going to be huge. It is easy enough to handle this in the database because it can be split into subcommands with temporary storage for intermediate results, but the point of the database is that your users shouldn't have to do it themselves.

2. If you don't take a string or some other portable query syntax, then you have to write programmatic interfaces in every language. Most popular database engines have libraries for many popular languages already, but we also know that, if it really comes down to it, we can write a basic communication method to get bytes into the database server process and use a database in any language we like. If you use a more complex expression syntax, the library that executes queries becomes much more complex. A user who uses a language that isn't supported no longer has a hope of quickly writing one, and unofficial libraries are likely to have trouble keeping up with additions to the syntax.

3. The method has existed for a long time and inertia is hard to fight. There are newer database engines that do what you say, but it won't be easy to convince everyone to dump SQL and adopt one of them with its unfamiliar syntax and incompatible behavior as the new standard, porting all applications that used it over.

[CowHorseFrog]

Re: Coders vs Developers

Impossible because parameterized statements are a software abstraction, they also just send strings with parameters within them.

[bombastic bob]

Re: Coders vs Developers

"String building SQL commands etc etc should just be an absolute no no."

You mean like constructing an 'INSERT" or "SELECT" command yourself? Works fine every time for me. Hard rules like that are what do not belong. Old school C code building queries and passing it to psql (or whatever) could be the best solution, even using 'malloc()/free()' on pointers, fixed length temporary buffers, and stdin/stdout to send and get data to/from a forked process that does the DBMS work, I do that a lot. It also means not having to link your code to a specific library, nor re-writing the interface too much if you switch DBMSs.

[Mike007]

Re: Coders vs Developers

Query("SELECT * FROM Somewhere WHERE Username ='$UserSuppliedVariable';")

Vs

Query("SELECT * FROM Somewhere WHERE Username = ?;", $UserSuppliedVariable)

Both allow the user supplied string to contain a ' but only one rewards said user with admin access.

[werdsmith]

Re: Coders vs Developers

SQL injection and how to mitigate is fundamental and has been known for decades.

It’s incredible that it’s still a problem on production software with database backends.

Put it in the test plan and fire any programmer that ignores it.

[CowHorseFrog]

Re: Coders vs Developers

Its not a shock its actually to be expected.

Just look at the hiring process, HR take the first shot at filtering applications. WTF does a HR person know about anything software engineering related ?

[John Miles]

Re: Coders vs Developers

and when you have them trained use parameterized statements, next thing you find is they are building xml/json by string concatenation (though I suppose should give them marks for using string builders) and passing those in as a string to stored procedure and processing that in the proc.

[Rich 2]

Nobody cares

I once ran a retail website for some time. It was based on open source code which I modified extensively to fix stuff and make it work the way i wanted.

During the inevitable visits to help forums and contacting various authors of this web stuff directly taught me that nobody gives a shit about security. If it’s there already then yea, why not use it? But if it’s not, most devs seem more interested in being able to change the colour of a button or track their customers then actually bothering to address the problem.

Incidentally, the same goes for the banks regarding credit card fraud. I contact my bank - The Bank of Scotland - to tell them about some dodgy credit card use I’d found in the logs. I had some useful details. They didn’t give a shit either.

[sitta_europea]

Re: Nobody cares

"... Incidentally, the same goes for the banks regarding credit card fraud. I contact my bank - The Bank of Scotland - to tell them about some dodgy credit card use I’d found in the logs. I had some useful details. They didn’t give a shit either."

Yeah, some years ago I found a problem with the password algorithm at Barclays.

For about six weeks their response was "No, you must be mistaken, it can't possibly be broken!".

But I kept at it, and eventually somebody there realized that I was right. Yes, it was broken, and they did finally fix it.

Afterwards Barclays' MD (at the time, a fellow named Diamond) called to thank me.

Unluckily for him our receptionist was trained to spot fraudsters impersonating officers at banks. She hung up on him. :)

[CowHorseFrog]

Re: Nobody cares

Its not that they dont care, its that they are blind and dont see the problems to begin with.

[Anonymous Coward]

Cost?

I notice comments here mention that the "cost" of securing software that "already works" is too high (where "too high" means "greater than zero").

Take SolarWinds as an example:

(1) The cost of a secure development environment (i.e. resistant to malware insertion) is some figure, say X dollars. SolarWinds thought X dollars was "too high".

(2) The costs faced by SolarWinds customers after the hack were almost certainly many many multiples of X dollars.

So......this sort of loose use of the word "cost" almost never mentions WHO ENDS UP DOING THE PAYING!!!

Exactly the same false logic occurs elsewhere - for example, banks exporting risk from their infrastructure....and exporting that risk to millions of customers.

....and the beat goes on...........

[gnasher729]

Make input sanitisation illegal. If I want to call my son “Bobby”;drop tables then I should be allowed to. Obviously if that crashes your application that’s your fault. I guess building queries by using string operations should also be illegal.

[bombastic bob]

"Bobby'; drop tables;" - heh.

You can store that within the table as "Bobby&#3b; drop tables" [only tolerating internal ';' when part of a '&#' sequence]. Or similar.

[ldo]

“parameterized queries with prepared statements” are “brittle”

Funny, that. Every time I post examples of how I dynamically construct SQL query strings with proper quoting of user input, I get yelled at and told to use “parameterized queries” and “prepared statements”, or even an ORM. Yet here we have the security experts saying that that is not a good solution, just like I thought all along.

[Doctor Syntax]

Re: “parameterized queries with prepared statements” are “brittle”

You're excused. The paragraph you're looking at uses "These" and "they're" ambiguously because it's not specified to what they refer.. You parsed them, quite reasonably, as referring to "parameterized queries with prepared statements" in the previous paragraph.. It seems more likely that "input sanitization techniques" was meant. If you read it like that it's consistent with your getting yelled at.

[ldo]

Re: more likely that "input sanitization techniques" was meant

“Input sanitization techniques” is that stupid thing I see in so much PHP code, where they laboriously go through and strip out everything they think of as a “problem” character. I don’t do that: I use proper string-quoting instead. Let the user type what they like; the database can handle it, and so can I.

[werdsmith]

Re: more likely that "input sanitization techniques" was meant

Can you handle ambiguity?

[ldo]

Re: Can you handle ambiguity?

Yes and no.

[Doctor Syntax]

Re: more likely that "input sanitization techniques" was meant

My GP's online system appears to be applying string sanitisation to passwords. That worries me they're storing the passwords online in clear.

[Dinanziame]

SQL is the problem

A lot of trouble could be avoided if people were using a real programming language to execute queries. SQL is the level of COBOL in language evolution, trying to have statements that sounds like English and ignore rules of logic. If people are not coding with statements like "ADD B TO C" they shouldn't be using "SELECT X FROM Y" either.

It's a travesty that database queries are transmitted by building a string from logic, and the string is then parsed back to logic. You might as well be printing them on punch cards on the way for all the sense it makes. Parameterized statements are just a bandaid to cover the flaws.

[Doctor Syntax]

Re: SQL is the problem

A good programming language is one which allows the developer to express the requirement clearly and should be equally clear to read for anyone who has to maintain the code afterwards. For its given domain SQL does just that and it's a good deal less verbose than what I understand COBOL to be.

I'm not sure of your preferred programming language but whatever it is could you give an example of how you think it should be done to extract data joining, say four tables, with selection criteria spanning at least five columns in at least three of the tables. At least one of the criteria should be a range of values and at least one other should provide the equivalent of a SQL OR clause to filter on multiple values for that column.

Then explain how the resulting code meets the clarity of expression and reading criteria better than the equivalent SQL.

The problem in TFA is not SQL It's in constructing the SQL on the fly as opposed to parametrising a pre-written statement. About the only benefit of your "real" programming language is that parametrising would be unavoidable. The downside is that you'd have to produce a complex set of statements for each individual SQL statement you replace.

I'm trying to cast my mind back 40 years to the time before Informix adopted SQL and we had to use a C library instead. Fortunately time is shielding me from that.

[ldo]

Re: constructing the SQL on the fly

Nothing wrong with constructing SQL on the fly. All the good DBMS APIs I’m aware of provide the necessary string-quoting functions to let you do this properly.

Actually, they only do that for literal data. I haven’t seen a single one that will deal with escaping wildcards for LIKE clauses. But it’s easy enough to roll your own.

[Dinanziame]

Re: constructing the SQL on the fly

But it’s easy enough to roll your own.

Famous last words...

[ldo]

Re: constructing the SQL on the fly

Here is a simple function that does conversion of a suitable set of Python objects to SQL strings:

def sql_string(s) :

␣␣␣␣"returns an SQL string literal that evaluates to s."

␣␣␣␣if s == None :

␣␣␣␣␣␣␣␣q = "null"

␣␣␣␣elif isinstance(s, int) :

␣␣␣␣␣␣␣␣q = "%d" % s

␣␣␣␣elif isinstance(s, (bytes, bytearray)) :

␣␣␣␣␣␣␣␣q = "X'" + "".join("%02x" % d for d in s) + "'"

␣␣␣␣elif isinstance(s, str) :

␣␣␣␣␣␣␣␣q = "'" + s.replace("'", "''") + "'"

␣␣␣␣else :

␣␣␣␣␣␣␣␣raise TypeError("can only convert bytes, str, int or None types")

␣␣␣␣#end if

␣␣␣␣return q

#end sql_string

As you can see, the checklist is actually quite short.

Feel free to point out what I’ve missed—if you think I’ve missed anything.

[doublelayer]

Re: SQL is the problem

"ADD B TO C" is fine. What we write now, c += b or c = c+b is no different semantically. The syntactic difference is unimportant. The problem is not with syntax that looks like English. The problem of injection is not unique to SQL or a language that looks like English, but is present in any language that can parse a generated string as code. Perl is notorious for this, because few Perl programs, at least the old ones when Perl was more popular, didn't use the eval function somewhere.

I wrote a comment in another thread above this one about why I think that SQL needs to look like it does here and accept a string query, because attempting to construct complex queries using a more traditional programming language either makes the code unreadable or makes the programmer do the work for the database (I.E. instead of running one query, running multiple ones and handling the intermediate stages manually). A language that looked like SQL but enforced parameterization could avoid the injection risks. One that looked more like code but didn't enforce them would be as vulnerable. One that looked like code and did enforce them could end up fixing the injection risk in such an ugly way that nobody used it.

[Doctor Syntax]

Re: SQL is the problem

"One that looked like code and did enforce them could end up fixing the injection risk in such an ugly way that nobody used it."

And would be more prone to bugs.

[xyz]

A salutory reminder...

That no matter how great and super dooper cool the next buzz thing is... at the end of the day it can all be bolloxed by a lazy query structure.*

*probably by a code firster who thinks databases are smelly.

[yaronf]

Misleading paragraph

This paragraph:

> These are deployed by some vendors, but were branded "brittle" by CISA and the FBI. They said they're also difficult to deploy on a large scale and are more easily bypassed.

Seems to refer to prepared statements in the preceding paragraph. Please re-read the PDF: it actually recommends using prepared statements rather than home-grown quoting of parameters (and the *latter* is called "brittle").

[Greybearded old scrote]

Re: Misleading paragraph

I also disagree with the claim that it's difficult to deploy. It was my default from the first db backed project I created. Damn tedious to retrofit when your predecessor (aka the nearly-technical director) didn't know/care about it.

If you're expected to accept arbitrary SQL from your users all bets are off of course.

[Doctor Syntax]

Re: Misleading paragraph

It seems to but doesn't. It can equally well, and should apply to string sanitisation. Not the finest bit of el Reg writing, I'm afraid.

[ldo]

Re: rather than home-grown quoting of parameters

The phrase used was “input sanitization techniques”. “Sanitization”, as I understand it, refers to the common practice, particularly among PHP programmers, of stripping out any characters that they deem “problematic”. This is, of course, a dumb thing to do. The right answer is to do proper string quoting, so that what the user input is exactly what they get. You need to do this in other situations, like for HTML output for example, so why not do it for SQL? It’s not that hard.

[ChoHag]

> The call from authorities extends to software vendors' customers too. They've been advised to hold their vendors to account by asking them if a formal code review

> Leaders must consider the full picture: That customers, our economy, and our national security are currently bearing the brunt of business decisions to not build security into their product

You go first.

[Greybearded old scrote]

Bobby tables

Obligatory. Also.

[ldo]

Re: Bobby tables

The irony behind your “Also” cartoon is that so-called “NoSQL” DBMSes have discovered that some form of SQL is a good query language even for nonrelational DBMSes—which is what they really are.

[Uplink]

Simple but complicated solution

The problem with SQL is that it's human-writeable text that can be concatenated on a whim. Solution: replace with binary protocol. Well... at least disallow direct coding of it - use builders.

Reasoning: if you want to keep humans away and let the computers build the safe commands, you make it really hard for humans to write it directly. Humans get to use a builder with that has safety built-in.

For example: Nobody is writing machine code. Nobody is really writing asm either. We use compilers. Make builders for SQL and Regex, and disallow direct string writing (the binary protocol is the sledgehammer that helps with this), and now the coders have to find a different way to introduce vulns without realising.