Stored procedures and ORMs won’t save you from SQL injection

This content is now available in the Pluralsight course "Ethical Hacking: SQL Injection"

Everybody knows the easiest way to save yourself from SQL injection is to use object relational mappers (ORMs such as Entity Framework) or stored procedures, right? Often I see this becoming a mantra: “You don’t need to worry about SQLi if you’re using [Entity Framework | stored procedures]”. I also see the mantra blindly repeated and it’s wrong, very wrong.

Of course this isn’t new to many people but it’s worth a recap of just how easy SQLi in poorly implemented code using ORMs or stored procedures. So let’s exercise some SQLi on an app that uses not one, but both of these!

Understanding the SQLi risk and mitigations

Let’s do a quick SQLi 101. Injection attacks work as an attacker is able to force the code to break out of a data context and begin manipulating the query context. What do I mean by this? Consider the following:

In a case like this, the query component is static in so far as it’s coded into the app whilst the data part is dynamic in that it’s provided by the user. When I say “provided by the user”, it comes from them via an HTTP request such as in a query string like ShowWidget.aspx?ID=1. The code in the web app would look something like this:

"SELECT * FROM Widget WHERE ID = " + Request.QueryString["ID"]

Of course because it’s provided by the user it’s also untrusted or in other words, we have no control over it and it might be used to do nasty things. SQLi happens when the users is able to do something like ShowWidget.aspx?ID=1 or 1=1 which, of course, would create a statement like this:

Why is this important? Because we’ve broken out of the data context and into the query context. The untrusted data provided by the user has changed the execution of the query not by adjusting a parameter (widget ID 1, widget ID 2, etc.) but by fundamentally changing its nature. Clearly in this case all records would be returned as the 1=1 or condition would always be true.

Selecting all records is (usually) innocuous but what this proves is that the attacker likely has the ability to change the query in other ways too. For example, they might terminate the statement and drop a table by doing something like ShowWidget.aspx?ID=1; DROP TABLE Widget or escalate privileges by doing something like ShowWidget.aspx?ID=1; INSERT INTO UsersInRoles… You get the point.

The most fundamental mitigation against this kind of attack is parameterisation. When we parameterise a statement, the query and the data are kept isolated. It means the query looks more like this:

"SELECT * FROM Widget WHERE ID = @ID"

And then the ID is passed as a parameter on the SQL command:

command.Parameters.Add("@ID", SqlDbType.Int).Value = id

That’s a very simplistic view but what it means is that the 1 or 1=1 untrusted data would be retained in a data context and the query would look for a record with an ID of that value. Actually, it wouldn’t even hit the database because the parameter above expects and integer, but you get the idea. Also, whilst that example is in C# .NET, other frameworks provide similar facilities.

The things about both stored procedures and ORMs is that they enforce parameterisation because usually, there’s no simple string concatenation of a query which is what went wrong in the first example above. Usually…

Injecting through a stored procedure and an ORM all in one go

Let’s take a scenario; you have a facility to search your widgets by name and naturally you have a search page where you can enter a widget search term. When you search, a page is loaded with the term in the query string, for example SearchWidgets.aspx?SearchTerm=widget

The code is then implemented as follows:

var searchTerm = Request.QueryString["SearchTerm"];
var db = new WidgetEntities();
var widgets = db.SearchWidgets(searchTerm);
WidgetGrid.DataSource = widgets;
WidgetGrid.DataBind();

This is simply an Entity Framework data context which has a stored procedure called SearchWidgets on it. The procedure takes a single string parameter and returns all the columns from the Widget table so that when it runs, it looks like this:

So this ticks all the boxes of why both ORMs and stored procedures provide a degree of native protection against SQLi, namely that the untrusted data – the search term – is parameterised and as far as the ASP.NET app is concerned, the data is handled properly.

So let’s make it interesting, let’s change the parameter to SearchWidgets.aspx?SearchTerm=shiny' or 1=1;--

BUT IT USES A STORED PROCEDURE AND AN ORM!!! IT’S MEANT TO BE SAFE!!! Yeah, about that…

The problem, of course, is that the underlying stored procedure is now doing the string concatenation with the query and data and allowing it to get all mixed up. Here’s what it looks like:

ALTER PROCEDURE dbo.SearchWidgets 
  @SearchTerm VARCHAR(50)
AS
BEGIN
  DECLARE @query VARCHAR(100)
  SET @query = 'SELECT Id, Name FROM dbo.Widget WHERE Name LIKE ''%' + @SearchTerm + '%'''
  EXEC(@query)
END

When the procedure runs with the untrusted input from earlier on, the value of the @query variable ends up looking like this:

SELECT Id, Name FROM dbo.Widget WHERE Name LIKE '%shiny' or 1=1;--%'

The SQLi payload has caused the where clause to simply return all records then comment out anything after the untrusted data. It’s as easy as that.

Does this actually happen?

Yes, frequently. This is a very simplistic example but certainly string concatenation within queries occurs. Usually it’s to achieve more complex tasks but you still end up with untrusted data being concatenated into a query which is where the real problem is. In the past, I’ve seen this risk manifested when the query syntax itself needs to be constructed from variables passed to the stored procedure, for example when running OPENQUERY to a dynamic Oracle linked server and programmatically building up the required columns and views based on the context the query is executed in.

Mitigations

The mitigations largely come back to material I’ve already written about in the OWASP Top 10 for .NET developers, part 1. In short:

1. The untrusted data needs to be validated against a whitelist of allowable values. The .NET code should be defining a regex of the characters it allows and that probably shouldn’t includes equals signs or semicolons.
2. The principle of least privilege should be applied to the SQL account used by the web app. Whilst it won’t stop the 1=1 sort of scenario, least privilege would mean things like not allowing the account used by public users to write to the UsersInRole table. Give it only the bare minimum it needs.
3. Avoid query concatenation at almost all costs. I know there’ll always be edge cases, but concatenating untrusted data directly into the query itself is the last thing you want to do anywhere. Most folks already know this is the case in the web tier, don’t forget it applies to stored procedures as well.

So that’s it – easy SQLi of an ORM and stored procedure so the next time you hear someone saying they’re “safe” because these feature in their data access, send ‘em here!