I couldn’t believe it when I read Julie Lerman’s post about EF and SQL Injection. She discovered that Entity Framework doesn’t use parameterized queries always!
I had to try it for myself on Northwind database:
var query = from c in entities.Customers
where c.ContactName == "Tubo"
select c;
produces this SQL statement (courtesy of SQL Profiler):
SELECT
[Extent1].[CustomerID] AS [CustomerID],
[Extent1].[CompanyName] AS [CompanyName],
[Extent1].[ContactName] AS [ContactName],
[Extent1].[ContactTitle] AS [ContactTitle],
[Extent1].[Address] AS [Address],
[Extent1].[City] AS [City],
[Extent1].[Region] AS [Region],
[Extent1].[PostalCode] AS [PostalCode],
[Extent1].[Country] AS [Country],
[Extent1].[Phone] AS [Phone],
[Extent1].[Fax] AS [Fax]
FROM [dbo].[Customers] AS [Extent1]
WHERE N'Tubo' = [Extent1].[ContactName]
See the Tubo condition expressed like a string? Speechless. Granted, if you specify a variable instead a string within the EF query then a parameter is used.
So I tried a classic SQL Injection attack, like this:
var query = from c in entities.Customers
where c.ContactName == "Tubo'; UPDATE [Northwind].Categories SET Description='SQL Injection' WHERE CategoryID=9; –'"
select c;
which produces this condition (select columns trimmed)
WHERE N'Tubo''; UPDATE [Northwind].Categories SET Description=''SQL Injection'' WHERE CategoryID=9; —''' = [Extent1].[ContactName]
Luckily the attack fails due to duplication of single quotes. So there is a protection against SQL attacks afterall.
However, I am not feeling safe with this approach. Attackers are smart and sooner or later they might find a way across string-replacement defense strategy.
But the real questions is: why leave the door open to a possible attack in first place? Why not always use parameters as there is no way around them? What good is to risk a SQL injection attack?
Update: A possible scenario where one could exploit this feature, granting there is a way around single-quote-replacement, is with add-ins. Imagine you have an add-in based application where each add-in is allowed to perform EF selects on database (enforced by your application). One could fake a select with all sort of T-SQL code nested within.
that is ridiculous. character replacement is called escaping and it is done with html as well to prevent xss. if your list of characters is exhausive, you are safe. in t-sql the only char in a string that has special meaning is the single tick. i once wrote a program to verify that (it send all possible combinations of 2 char (that 4 billion) to a sql server. only the ones with ticks failed.
I don’t think it is that ridiculous. The fact is that if you use parametrisation then you are safe against sql injection attacks. If you don’t then you *might* be safe.
Hi Miha
What I was showing is that if you are hardcoding the LINQ query, then you can hardcode in the injection. Though when/why would y ou do that? If you are using variables in the LINQ query, then the store query will be parameterized to protect you from whatever might be in that variable. LINQ queries are safe.Take a look at Danny Simmon’s followup post and see if it doesn’t make you feel better. blogs.msdn.com/dsimmons
julie
Hi Julie,
I wouldn’t hardcode it but a malicous developer might somehow do it. No, I don’t know how to do it. But somebody might find a way eventually. Why would one leave even a slightest possibility? Imagine the scenario when you are dealing with add-ins. You might oversee such hardcoded sql injection.
Anyway I am still failing to see the reason to not use parameters – what are the benefits?
Parameters are definitely better. However you can’t replace all dynamic query construction with parameters. For example if you wanted to dynamically build a projection:
“select ” + propertytoselect + “from AWEntities.Customers as ” + definingvariable
you can’t do this with parameters
Miha, parameters also have cost of their usage. Why would I parametrize something that I know to be constant ?
On the other hand it seems to me that if an developer can execute a L2S query than he probably can execute standard ADO.NET queries also an do damage that way. I do not see EF as a security layer.
If we want to secure data from devs maybe we should fall back to SP ? 🙂
One possibility would be for an attacker to alter the strings inside an assembly to perform an injection attack. Not very likely I know but one way to exploit this.