14 Years of SQL Injection and still the most dangerous vulnerability
mavitunasecurity.com
mavitunasecurity.com
Also:
This is primarily because of the most obvious problem:
We are still using relational SQL databases.
Were we to use NoSQL database systems such as MongoDB
or CouchDB, none of these attacks would ever happen
Yeah, and motorbikes are safer than cars because the third and fourth wheels never fall off...:( We are not safe
Why it's so common is that you can easily connect components incorrectly, still have it "work" enough to do a demo, and, there is lot of pressure to put things that appear to work into production without auditing them.
Addendum: Shish2k observes this article claims NoSQL databases would somehow solve the problem, rather than seeing NoSQL as just a different kind of component with it's own escaping/validation needs. Classic -- someone writing on this very topic is blinded to the phenomenon he discusses; the article itself demonstrates why we have SQL injections.
Something about that sounds wrong. It is as though you are suggesting that we use in-band signaling with a bunch of notch filters to ensure that Cap'n Crunch whistles cannot be used to get free calls. The right answer is out-of-band signaling -- in other words, not constructing queries / command strings / etc. from user inputs.
Major SQL databases all support prepared statements; this seems like a far more robust approach than trying to sanitize inputs.
Imagine constructing a function in some other language this way:
function square(x) {
code = x + "*" + x;
return eval(code);
}
This is obviously wrong. Now, let's say I suggest fixing it with escaping: function square(x) {
code = escape(x) + "*" + escape(x);
return eval(code);
}
Any sane programming community would string me up by my thumbs for doing this, and rightly so.Yet, do the exact same thing in SQL and it's just business as usual. Occasionally people talk about prepared statements and parameterized queries and such, but usually people just talk about escaping. As if the problem is just that you need more backslashes, not that you're glomming strings together at runtime and then calling eval() on them.
It's so bizarre. People would think you're crazy for doing it in one environment, but doing it in a different environment where the exact same problems are present is just fine! I can't fathom how we got to this place.
Occasionally? For many years "preferred prepared statements and use user input, sanitized (via escaping and/or more involved means) or not, only with a very special need that where prepared statements don't do what you need, and then be as restrictive as possible in what you accept" has been what I've heard everywhere.
To double-check and make sure I'm not just being biased, I did a Google search for "php mysql tutorial" and read through the top five results. Not a single one mentioned parameterized queries. One of the tutorials didn't mention using foreign data in queries at all, just hard-coded query strings. Of the other four, two escaped parameters and two just put everything in single quotes and called it a day.
Java, Perl, Ruby, Python, C#, NodeJS and virtually other language or framework strongly encourages the use of placeholders. PHP's community stands alone in stubborn opposition to this despite having facilities to do this.
PHP is also unusually hostile to frameworks and ORM-like database layers even when these would solve a myriad of problems without imposing too greatly.
Honestly, on the whole PHP programmers in general are like hunter gatherers who see no use for anything other than the most basic of tools. There are exceptions, but I figure these people usually graduate to other communities where the best practices they follow have already been fully embraced.
I have no problem acknowledging that the dominant advice in the domain of PHP MySQL tutorials has been to use escaping, and that that's a serious problem.
Clearly, we were thinking about different scopes.
I'd note for database languages, query construction is often two phase, where you build your query template based on selection/filtering needs, and then, use parameter substitution with user-provided content. I've seen experienced programmers do both at the same time and resort to manual escaping of user content rather than seeing them as distinct phases.
I think it's about managing the movement of information between contexts... one process' data is another process' code. The most important context switch happening between untrusted inputs and the rest of your system.
I apologize for deleting my other comment; I was trying to shorten it.
You make good points, and I'm trying to figure out exactly how your points and mine hook together.
I think what it ultimately comes down to is distinguishing between data and code. More specifically, data can contain code, but code should never contain (foreign) data. So escaping in general is fine, but escaping in this context, where you're building a string of code, is no good.
That, I think, is why I'm comfortable with e.g. the escaping done to a string when emitting JSON, but not with escaping a string to put it into an SQL query.
With parameterized queries, you're still ultimately passing the code and data over to the database over some sort of stream (assuming an out-of-process database server), but hopefully that stream is designed as data containing both the parameterized SQL code and the parameters, rather than just SQL code with escaped parameters.
SELECT * FROM bar_table WHERE x = 123;
Is converted into a datastructure like this in the database engine (really simplified): {
"querytype": "select",
"fields": ["bar_table.x", "bar_table.y", "bar_table.z"],
"tables": ["bar_table"],
"where_filters": ["x", "=", 123]
}
Which is then converted into a list of things to do to run that query.Now, if you give it a prepared statement:
SELECT * from bar_table WHERE x = ?;
The database engine can convert that into a closure, something like this: function(arg1){
return {
"querytype": "select",
"fields": ["bar_table.x", "bar_table.y", "bar_table.z"],
"tables": ["bar_table"],
"where_filters": ["x", "=", arg1]
};
}
Then when you run the prepared query, it can simply call the closure, putting the right value in the datastructure without escaping anything, and continuing on from there.Of course, the realities of it are more complicated than that for optimization reasons, but that's the basis of it.
How do you explain shell scripting then? Bash and co. have been 'industry best practice' for decades. They feature all these problems of in-band communication and (attempts at) escaping and nobody seems to consider it a problem.
SQL statements (unparameterized) have a poor distinction between what is an element of the SQL language and what is permitted data input: where that boundary can be crossed, injection attacks become possible.
Well, no, its done by constructing outputs from inputs in one module, and validating inputs in the next module (you can validate outputs, as well, but that's less critical.) "Escaping" is one method of constructing outputs from inputs, but its often an unsafe and undesirable method.
It's not that devs and DBAs don't care, it's that in many cases (not all) they aren't given the time to carefully test the system to see how it might be abused.
And also, many companies view security testing as an afterthought or an audit requirement. If they do not have to do it (time is money) they won't. And when they are forced to do it, they rush through it.
In some ways this is a philosophical fight between a group that wants to ship software and make a profit and a group that wants to design and test systems to make sure they are correct. Suits vs technologists.
$stmt = $dbh->prepare("SELECT * FROM users WHERE USERNAME = $username AND PASSWORD = $password"); $stmt->execute();
vs
$stmt = $dbh->prepare("SELECT * FROM users WHERE USERNAME = ? AND PASSWORD = ?"); $stmt->execute(array($username, $password));
adds no time or effort. SQL Injection happens because devs haven't been taught to do the right thing or are too dumb to remember to do the right thing. Both conditions are easy to fix: teach the first group, fire the second.
This is not software engineering alone, by the way.
User input and SQL statements are two completely separate things, but it just so happens that they can both be represented in memory using the same layout. Hence many developers stop thinking in meaningful terms like "user input" and "SQL query", and instead start thinking in terms of implementation details, like "strings".
The solution to code injection is not to jump on the bandwagon du jour, but to maintain and enforce distinctions between meaningful datatypes. See, for instance, http://blog.moertel.com/posts/2006-10-18-a-type-based-soluti...
"Systems Hungarian" has the same cause; Hungarian notation means prefixing variable names with useful information about the contents, for example "dAge" could be the difference between two ages and "cUsers" could be a count of how many users there are. When this practice became enforced at, for example Microsoft, it quickly degraded into the meaningless "Systems Hungarian" which just gives unhelpful implementation details, eg. "iAge" and "iUsers" to indicate that they're both integers. Widespread use of Systems Hungarian, enforced by clueless management, lead to many developers hating Hungarian notation completely, despite never knowing that it has nothing to do with machine representations.
How would an ideal injection-free application look? I'd imagine a set of unmutable precompiled SQL statements (the code) each controlled by a set of parameters (the data.) No gluing of statements from strings at runtime and the parameters are obviously passed out-of band. You can't forget to escape user-provided data because in this setup the SQL code and the user data never mix, so escaping is not necessary.
Unfortunately is not feasible to use the above design with SQL. A simple filter with a parameter that can be unspecified (as in "don't care") would require 2 different precompiled statements ("select ... from foo" and "select ... from foo where param=<placeholder>".) The number grows exponentially with the complexity of the filter, so the runtime construction of SQL statements is inevitable.
You can go about it in a smarter or a dumber way but the danger remains.
You can still construct at runtime. Just let your typesystem handle the distinction between SQL code and data.
* By 'untyped' I mean that the type-system that you were using is no longer enforcing anything. There may be another type system in the compiler target.
However, we don't need to do anything nearly so elaborate to stop SQL injection. We just need a type "SqlQuery", since that will be instantly unusable by all string concatenation functions. We then make a concatenation function for SqlQuery values and a "stringToSqlQuery" function (or ".toSqlQuery" method, if you prefer) which converts strings to SqlQuery values by escaping them.
This way, we've turned SQL escaping into a type coercion, so we can only pass the type checker by escaping every string we put in our queries. Note also that it solves the double-escaping problem: since escaped strings have a different type to unescaped strings, we can't send them back through the escape function; ie. "stringToSqlQuery(stringToSqlQuery(foo))" is a type error.
$params = array();
$sql = "select * from foobars where 1";
if (isset($_GET['name'])) {
$sql .= " and name = :name"
$params[':name'] = $_GET['name'];
}
$stmt = $db->prepare($sql);
$stmt->execute($params); UserInput readFromPost(HttpParam param) {
// Read param from POST here
}
Collection<SqlRow> query(SqlQuery q) {
// Send query to DB here
}
UserInput name = readFromPost('name');
Collection<SqlRow> result = query(
sqlConcat('INSERT INTO users (name) VALUES (',
stringToSql(userInputToString(name)),
')')
SqlInt<10> userId = sqlRowLookup(result, 'id')
sendToBrowser(htmlConcat('You are user number ',
sqlIntToString(userId)))
Clearly we can get better APIs than this, but it's not as difficult as you make out to program in a safe and sane way.Binding data to a query after the fact is a reliable way of escaping. Composing a prepared statement is not hard, and creating these at runtime is not a big deal.
Honestly, for most of your daily work you should be using an ORM of some kind where this is already done for you. The only reason you should be writing direct SQL is when you're doing things the ORM doesn't natively support.
Ruby on Rails is currently 100% injection free as far as anyone can tell, and if it isn't there'd be an emergency patch issued right away. There have been situations in the past where this was not the case, but these bugs were quickly addressed after being discovered. Most other frameworks (Django, etc.) are in the same category.
SQL q = <| SELECT id FROM users WHERE name = ${userName} AND password = ${password} |>;
The <|...|> construct switches from Java to SQL, while ${...} switches from SQL to Java. The desugaring process that translates the Java+SQL to plain Java ensures that escaping is done at the right places. This approach is based on SDF2, a language for defining grammars that allows context-free grammars to be combined in almost arbitrary ways.This kind of stuff always seems to surface when one language is embedded in another and this tends to be database query languages and one of the most popular is SQL, but it's not limited to it, just look at javascript injection from parsing JSON using eval.
I am not seeing injection going away until we stop sending bits of code between runtimes as strings, which will be hard to avoid so long as databases and applications use different languages. It's a lot of work to abstract one language into the other (See LINQ to SQL), where as string concatenation is understandably the easy way out.
Ideally we could all agree on a good binary data format(something like Go GOBs) and a AST structure to serialize to it then we would have the common building blocks to send dynamically built code between runtimes/languages without using strings.
No.
Its about lack of care in building executing code from untrusted input. If you took data from the outside world, converted it into x86 machine code, and sent to it another system, you could still have the same problem if you didn't handle the input properly.
SQL-by-string-interpolation/concatenation is an easy target because its a common enough mistake that the same style knowledge of how to exploit the holes in it can be used against lots of targets, not because the transfer format from the poorly-built system generating the query to the critical backend system handling the query uses strings.
We actually use a component in our app the builds an AST from SQL which can then be verified, this is not trivial. Also LINQ builds an AST which is then transformed to SQL statements in LINQ to SQL, this AST can also be examined before executed and prevents injection.
("Must be secure" is a much higher level requirement than anything that is testable, but a high level requirement is meaningless except to the extent its operationalized into lower-level requirements that are testable -- or analytically provable, but that's even harder.)
Because most of the products are developed by the lowest bidder.