I'm not sure what your URL is showing? The ask was to make it so those have to be supplied as a parameter to the sql, such that all of those would be considered literals if put directly in the query. Right?
More directly, ad-hoc query generation is something that shouldn't be allowed from potentially hostile users. Which, by and large, means all users that don't otherwise have admin access to the system.
What most people mean by query parametrisation in my example (as pseudocode) is something like the following:
conn.Query( "SELECT * FROM Foo WHERE id BETWEEN @From AND @To ORDER BY " + HttpRequest["sort"],
new { From: HttpRequest["from"],
To: HttpRequest["to"] });
In typical SQL or DB client libraries you can parametrise literals, but not queries. The queries are almost always built up as strings. Any time you see "sort=DESC" in a URL, there's like a 90% chance that code like the above is floating around in the codebase, just waiting to be exploited. That's because there's no way to use a safe parameter for "sort order", or "which columns to sort by", or... anything else for that matter. You just have be eternally vigilant. And by "you", I mean everyone everywhere.
Thankfully, most pen-testers cackle with glee when they see "DESC" or "ASC" in a URL, but what if the code above does some sort of simple check to see if the string is one of the allowed two values?
... I can tell you what happens: the pen test report will be ignored as "not actually vulnerable", and now you have landmine in the code. Any junior dev updating or rewriting the queries may forget the sanity check and... Boom!
Building up queries as strings is a pit of failure into which developers have been falling en-masse for decades and decades. Millions of them. Entire industries and products have been built out to detect, mitigate, and protect from this pit.
What we need is something more like the bytecode used by the JVM or the .NET Framework. A set of binary primitives that must be built up via a fluent API such as:
Even if "OrderBy()" allows a string input, it'll just be doing the same kind if parsing logic instead of just "appending" to the end of the query blindly.
Ultimately if your language supports constructing strings from untrusted input + a query language, you have an injection problem. The question is mostly one of scope. As an example, CQL doesn't have nearly the same power of SQL, and so an injection is just not as big of a deal - it's still possible, and in some cases it could be bad, but it's not nearly the same kind of primitive as SQL.
Similarly, removing primitives would probably solve a lot of injection use cases, but if someone does something insane like inject DESC into their query, it's over.
Because SQL is so powerful it's really just up to the libraries to try to help where they can, we'll never truly be rid of injection vulns as they're fundamental to any program that treats data as code.
But, I honestly see very little reason to allow dynamic strings in building queries. Such that I would happily take a system that failed the build if you passed a non static string to the query constructor. I thought this was common with build systems and format strings?
u/jiggawatts is saying that you or others can always build a thing (a language, in this case URI q-params) that is itself vulnerable to injection attacks even if under the covers you were using an injection-proof SQL.
This is obviously true. In that sense my statement that u/jiggawatts was responding to is in some way very much incorrect. But my statement was specifically about SQL injection, and not about things one might build with an injection-proof SQL.
Comments
I'm not sure what your URL is showing? The ask was to make it so those have to be supplied as a parameter to the sql, such that all of those would be considered literals if put directly in the query. Right?
More directly, ad-hoc query generation is something that shouldn't be allowed from potentially hostile users. Which, by and large, means all users that don't otherwise have admin access to the system.
What most people mean by query parametrisation in my example (as pseudocode) is something like the following:
In typical SQL or DB client libraries you can parametrise literals, but not queries. The queries are almost always built up as strings. Any time you see "sort=DESC" in a URL, there's like a 90% chance that code like the above is floating around in the codebase, just waiting to be exploited. That's because there's no way to use a safe parameter for "sort order", or "which columns to sort by", or... anything else for that matter. You just have be eternally vigilant. And by "you", I mean everyone everywhere.Thankfully, most pen-testers cackle with glee when they see "DESC" or "ASC" in a URL, but what if the code above does some sort of simple check to see if the string is one of the allowed two values?
... I can tell you what happens: the pen test report will be ignored as "not actually vulnerable", and now you have landmine in the code. Any junior dev updating or rewriting the queries may forget the sanity check and... Boom!
Building up queries as strings is a pit of failure into which developers have been falling en-masse for decades and decades. Millions of them. Entire industries and products have been built out to detect, mitigate, and protect from this pit.
What we need is something more like the bytecode used by the JVM or the .NET Framework. A set of binary primitives that must be built up via a fluent API such as:
Even if "OrderBy()" allows a string input, it'll just be doing the same kind if parsing logic instead of just "appending" to the end of the query blindly.I mean, fair on the sort order. I was distracted by the other items.
But there is no reason that couldn't be made one of the things that can't be included as a literal. Is there?
Ultimately if your language supports constructing strings from untrusted input + a query language, you have an injection problem. The question is mostly one of scope. As an example, CQL doesn't have nearly the same power of SQL, and so an injection is just not as big of a deal - it's still possible, and in some cases it could be bad, but it's not nearly the same kind of primitive as SQL.
Similarly, removing primitives would probably solve a lot of injection use cases, but if someone does something insane like inject DESC into their query, it's over.
Because SQL is so powerful it's really just up to the libraries to try to help where they can, we'll never truly be rid of injection vulns as they're fundamental to any program that treats data as code.
But, I honestly see very little reason to allow dynamic strings in building queries. Such that I would happily take a system that failed the build if you passed a non static string to the query constructor. I thought this was common with build systems and format strings?
But then where do you stop?
Fundamentally the problem is with any kind of dynamic query construction. It’s very hard to make this secure and general purpose.
I'd happily not allow any dynamic query construction. Just as I wouldn't allow dynamic format string construction.
u/jiggawatts is saying that you or others can always build a thing (a language, in this case URI q-params) that is itself vulnerable to injection attacks even if under the covers you were using an injection-proof SQL.
This is obviously true. In that sense my statement that u/jiggawatts was responding to is in some way very much incorrect. But my statement was specifically about SQL injection, and not about things one might build with an injection-proof SQL.
It's a quibble, though an important one.