7

I know that if I use linq to sql, everything will be parametrised and sql injection safe. But how about IQueryable?

For example, I can cast some entity to Iqueryable:

var myquery = mytesttable.AsQueryable();
var qText = "name="+ "\""+DynamicSearchCondition+ "\"";
myquery = myquery.Where(qText);

Then when the query is run, from trace I can see that the DynamicSearchCondition passed in is not parametrised.

Initially I thought this is not sql injection proof, but I then tried some examples, and just can't break this one. Does it mean it is sql injection free then (I think it is now)?

If that is true, will it mean all IQueryable are sql injection safe?

Rob Sedgwick
  • 4,342
  • 6
  • 50
  • 87
daxu
  • 3,514
  • 5
  • 38
  • 76
  • I don't think its safe see this blog `http://blogs.msdn.com/b/publicsector/archive/2008/08/21/preventing-sql-injection-with-the-entity-framework-and-data-services.aspx` – johnny 5 Oct 02 '15 at 14:18
  • Correct link is: http://blogs.msdn.com/b/publicsector/archive/2008/08/21/preventing-sql-injection-with-the-entity-framework-and-data-services.aspx – Rob Sedgwick Oct 02 '15 at 14:25
  • where you get `Where` with argument _string_? – Grundy Oct 02 '15 at 14:40
  • It probably escapes '. Did you see if it made changes to prevent infection attacks. Clearly parametrised queries is the best practice. – paparazzo Oct 02 '15 at 16:06
  • where argument comes from UI, which means it opens to potential issue. – daxu Oct 02 '15 at 16:07
  • 1
    @daxu, if you use DynamicLinq, possibly [this post](http://stackoverflow.com/questions/21582529/safe-dynamic-column-name-in-dynamic-linq/21772352#21772352) helps – Grundy Oct 05 '15 at 07:02
  • for an equal compare, seems it is safe as well. Just want to see if other people agree as well. – daxu Oct 05 '15 at 08:44
  • 2
    Possible duplicate of [Is Injection Possible through Dynamic LINQ?](http://stackoverflow.com/questions/8738953/is-injection-possible-through-dynamic-linq) – Robert McKee Oct 14 '15 at 21:01

2 Answers2

2

Absolutely it is vulnerable to injection attacks.

For your particular example:

var myquery = mytesttable.AsQueryable();
var qText = "name="+ "\""+DynamicSearchCondition+ "\"";
myquery = myquery.Where(qText);

would fail with this:

var DynamicSearchCondition= "\" or \"\"=\"";
Robert McKee
  • 21,305
  • 1
  • 43
  • 57
  • Technically, I suppose it could be argued this isn't a SQL Injection, but rather a DynamicLinq predicate injection attack, but the "look and feel" is the same, with similar concerns. Since DynamicLinq's syntax is essentially SQL (or it's own flavor of SQL), you could also argue that it still is an SQL injection, just one that gets translated into an expression tree, then back to SQL again. – Robert McKee Oct 14 '15 at 21:12
  • In either case, it is susceptible to the more general injection attack no matter how you view it. – Robert McKee Oct 14 '15 at 21:17
  • They are similar concerns, and should absolutely be taken into consideration, but the question is about SQL injection, referring to parameterization of the query. – Ocelot20 Oct 14 '15 at 22:44
  • @Ocelot20 It is still SQL injection. Please give a definition of SQL injection that this fails to meet. Was unwanted/unplanned SQL inserted into the (final) query? Yes. It does prevent the common `DROP TABLE` type injections, but there are other security concerns that would allow similar issues. If DynamicSql is used for logins for example, you could craft a username that would allow anyone to log in as an administrator, for example. – Robert McKee Oct 15 '15 at 15:44
  • @Ocelot20 You could inadvertently allow access to view records that you shouldn't be allowed to access, or update/delete records you shouldn't be allowed to access (Change everyone's password to 'blah', including the admins, or grant yourself admin rights). Assuming other restrictions are all in the same `Where` clause. – Robert McKee Oct 15 '15 at 15:48
0

No, IQueryable itself is not injection proof because it is just an interface for building a query Expression. It does not define how to take that Expression and turn it into something to be executed, such as SQL. What does perform this is the query Provider (many exists. Linq to Objects, Linq to Entities, Linq to Excel, to name a few).

That said, your example which seems to use DynamicLinq (based on the .Where(string) extension use) should have similar parameterization protections as regular Linq to Entities IQueryable's. DynamicLinq doesn't introduce any additional SQL injection concerns, because it's just a utility that works on top of IQueryable. Everything it does is just translated to an expression tree that again depends on the Provider to actually translate to SQL. That doesn't mean that the DynamicLinq syntax itself is safe from its own injection potential (see here for some examples, but these are not SQL injection).

Microsoft has this to say about LINQ to Entities and SQL injection:

Security Considerations (Entity Framework)

Although query composition is possible in LINQ to Entities, it is performed through the object model API. Unlike Entity SQL queries, LINQ to Entities queries are not composed by using string manipulation or concatenation, and they are not susceptible to traditional SQL injection attacks.

This means your DynamicLinq built IQueryable (if using LINQ to Entities as the provider) should still parameterize inputs. If your question is really "Is LINQ to Entities injection proof?", then the best answer I could give is "Probably. They have made all reasonable efforts to protect against it.".

Community
  • 1
  • 1
Ocelot20
  • 10,510
  • 11
  • 55
  • 96
  • DynamicLinq **DOES** introduce additional SQL injection concerns, because it is composed by using string manipulation and/or concatenation, so it **IS** susceptible to traditional SQL injection attacks. – Robert McKee Oct 14 '15 at 21:05
  • No it doesn't, and I even included examples of the difference between SQL injection and DynamicLinq injection. Can you please give me an example to back your claim? – Ocelot20 Oct 14 '15 at 22:33
  • See answer above, or see the duplicate answer I've indicated. – Robert McKee Oct 15 '15 at 15:05