No it wouldn't! I regularly see web apps with URLs like this:
someapp/reports/foo?start=1000&end=2000&sort=DESC
There are two fixes for this that mostly work:
1) Use a binary-only format that has no string representation at all. Instead, queries must be built up using a fluent builder style, or similar API surface. This is fine for "apps", but is less useful for ad-hoc queries obviously.
2) Use something similar to what web app frameworks do when generating HTML. Instead of accepting naked 'string' types, only accept something like an 'IEncodedHtml' type that automatically converts from strings, but escapes them in the process. Passing in a raw string as SQL would require some sort of scary-looking function call that is easy to grep in a codebase. E.g.: "QueryBuilder.UnsafeConvertTrustedString(...)" to really drive the point home. And then of course, IDEs and build tools would need to emit warnings or outright errors if HTTP inputs are directly passed to this function as-is.
Having said that, nothing will ever be sufficient. Developers start sentences with: "I just want to...", which is basically to say that they don't want to have to "deal" with security and just get it "working".
Another GIANT mistake I see in most (all?) database clients is that there is no distinction between needing a read-write connection or a read-only connection.
Making read-only the default would have huge benefits, such as:
1) Making all apps automatically gain the ability to do read-scale-out when using systems such as SQL Server AlwaysOn Availability Groups with readable secondaries. Right now this takes a major application code change to have any benefit, which means "nobody" does it. I've been literally begging and pleading with developers to just add a second connection string with "ApplicationIntent=ReadOnly" in it for report generation, and NOBODY has done it. Not one developer.
2) Security! If you don't need your app, page, or specific query to write, then don't enable the write privilege! Again, the default here is super dangerous, but every developer does it because it's the easy happy path. I'd be shocked if even 1% of the web applications in the wild switch to a read-only account or connection string, because it is so fiddly in current systems.
Even with all of the above, databases won't be perfectly secure against injection. The latest SQL injection attacks I've seen pen testers use is to add "DELAY" commands to exfiltrate data one bit at a time. This works similarly to micro-architectural side-channel attacks such as the famous Spectre and Meltdown vulnerabilities in Intel. Even if explicit injection isn't possible, timing attacks are often still possible if ad-hoc query generation is exposed.
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.
You can build an injection-vulnerable language on top of an injection-proof, so in that sense, injection issues can't fully go away. So you're quite right. But it's harder to build an injection-vulnerable language than it is to use an injection-proof language, so having an injection-proof language should help reduce injection vulnerabilities.
Comments
No it wouldn't! I regularly see web apps with URLs like this:
There are two fixes for this that mostly work:1) Use a binary-only format that has no string representation at all. Instead, queries must be built up using a fluent builder style, or similar API surface. This is fine for "apps", but is less useful for ad-hoc queries obviously.
2) Use something similar to what web app frameworks do when generating HTML. Instead of accepting naked 'string' types, only accept something like an 'IEncodedHtml' type that automatically converts from strings, but escapes them in the process. Passing in a raw string as SQL would require some sort of scary-looking function call that is easy to grep in a codebase. E.g.: "QueryBuilder.UnsafeConvertTrustedString(...)" to really drive the point home. And then of course, IDEs and build tools would need to emit warnings or outright errors if HTTP inputs are directly passed to this function as-is.
Having said that, nothing will ever be sufficient. Developers start sentences with: "I just want to...", which is basically to say that they don't want to have to "deal" with security and just get it "working".
Another GIANT mistake I see in most (all?) database clients is that there is no distinction between needing a read-write connection or a read-only connection.
Making read-only the default would have huge benefits, such as:
1) Making all apps automatically gain the ability to do read-scale-out when using systems such as SQL Server AlwaysOn Availability Groups with readable secondaries. Right now this takes a major application code change to have any benefit, which means "nobody" does it. I've been literally begging and pleading with developers to just add a second connection string with "ApplicationIntent=ReadOnly" in it for report generation, and NOBODY has done it. Not one developer.
2) Security! If you don't need your app, page, or specific query to write, then don't enable the write privilege! Again, the default here is super dangerous, but every developer does it because it's the easy happy path. I'd be shocked if even 1% of the web applications in the wild switch to a read-only account or connection string, because it is so fiddly in current systems.
Even with all of the above, databases won't be perfectly secure against injection. The latest SQL injection attacks I've seen pen testers use is to add "DELAY" commands to exfiltrate data one bit at a time. This works similarly to micro-architectural side-channel attacks such as the famous Spectre and Meltdown vulnerabilities in Intel. Even if explicit injection isn't possible, timing attacks are often still possible if ad-hoc query generation is exposed.
Security is hard.
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.
You can build an injection-vulnerable language on top of an injection-proof, so in that sense, injection issues can't fully go away. So you're quite right. But it's harder to build an injection-vulnerable language than it is to use an injection-proof language, so having an injection-proof language should help reduce injection vulnerabilities.