How can I secure SQL parameters with entity framework SqlQuery?

Question

I have this method that should take an unknown amount of id's. I got this method almost done but it isnt secure yet for obvious reasons, i know i could write my own method to strip the parameters but i would be more comfortable by using some build in method for this.

Here is the method

public static List<LocationModel> FetchCitiesByAreas(IEnumerable<string> areas)
    {
        using (var db = new BoligEnt())
        {
            var sqlQuery = new StringBuilder();
            var first = true;

            sqlQuery.Append("SELECT DISTINCT a.city AS City, a.zip AS Zip ");
            sqlQuery.Append("FROM zip_city AS a ");
            sqlQuery.Append("WHERE country = 1 ");

            foreach (var d in areas)
            {
                if (first)
                {
                    sqlQuery.Append("AND a.area_id = '" + d + "'");
                    first = false;
                }
                else
                {
                    sqlQuery.Append("OR a.area_id = '" + d + "'");
                }

            }

            return db.Database.SqlQuery<LocationModel>(sqlQuery.ToString()).ToList();
        }
    }

i know it have this function built in but as i stated earlier i dont know the exact amount of ids that will come in

db.Database.SqlQuery<LocationModel>("SELECT * FROM table WHERE id = @p0 ;", id).ToList();

Thanks

I think you're kinda missing the point of EF - which is: avoid having to write all the SQL, but instead use nice Linq queries to get your data that you need..... — marc_s
– marc_s, Commented Oct 1, 2014 at 20:43
As it is, there is a flaw in your sql. you dont put parens around your OR statements, so you'd get results where country does not equal 1. — crthompson
– crthompson, Commented Oct 1, 2014 at 20:46
thanks didn't see that, yes i know i can use linq but this is a simplified version of an original method in the original method there is some joins and entity framework doesn't handle it as good as raw sql in this case — Mikkel Refsgaard
– Mikkel Refsgaard, Commented Oct 1, 2014 at 20:56

entropic · Accepted Answer · 2014-10-01 21:04:55Z

3

While I completely agree with paqogomez, in that you should just use LINQ to do the query, the .SqlQuery has the ability to take a parameter array. You could change your statement to look like this:

var sqlQuery = new StringBuilder();

sqlQuery.Append("SELECT DISTINCT a.city AS City, a.zip AS Zip ");
sqlQuery.Append("FROM zip_city AS a ");
sqlQuery.Append("WHERE country = 1 ");

for (int i = 0; i < areas.Count; i++)
{
    if (i == 0)
    {
        sqlQuery.Append("AND (a.area_id = @p" + i.ToString());
    }
    else
    {
        sqlQuery.Append(" OR a.area_id = @p" + i.ToString());
    }
}
sqlQuery.Append(")");

var results = db.Database.SqlQuery<LocationModel>(sqlQuery.ToString(), areas.ToArray()).ToList();

I added the missing parenthesis needed to your query to correctly filter out the OR results as well. I've also taken the assumption that areas is something like a List, or at least something you can easily get the count from.

answered Oct 1, 2014 at 21:04

entropic

1,68314 silver badges27 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

crthompson Over a year ago

Excellent answer, I like your for loop idea. +1

Mikkel Refsgaard Over a year ago

Thanks man yes area is just a list<string> so it can be counted, it works great thank you very much.

crthompson · Accepted Answer · 2014-10-01 21:22:38Z

3

Why dont you just use Linq?

var locations = (from zip in db.zip_city
                 where areas.Contains(zip.area_id) && zip.Country == 1
                 select new LocationModel{
                   City = zip.City,
                   Zip = zip.Zip
                  })
          .Distinct()
          .ToList();

If you still want to parameterize your query, you need to use EntityCommand

Also note that your query will fail because you havent put parenthesis around your OR statements.

I suggest structuring your sql like this:

string sqlQuery =
    @"SELECT DISTINCT a.city AS City, a.zip AS Zip
         FROM zip_city AS a 
         WHERE country = 1 AND (1=0 "

   for (int i = 0; i < areas.Count; i++)
   {
       sqlQuery.Append("OR a.area_id = @d" + i.ToString() + " ");
   }
   sqlQuery.Append(")");

edited Oct 1, 2014 at 21:22

answered Oct 1, 2014 at 20:43

crthompson

15.9k6 gold badges64 silver badges87 bronze badges

3 Comments

Mikkel Refsgaard Over a year ago

Yes i know i can use linq this method i posted is a simplified version of the exact method. The original method have allot of joins and stuff and entity framework doesn't handle it as good as i want it to, i get a great performance boost by doing it in raw sql

entropic Over a year ago

Clever trick on the for loop to avoid the if statement - but shouldn't that be 1 = 0? 1 = 1 would evaluate to true and totally ignore all the other OR statements...

crthompson Over a year ago

@entropic you are correct, OR's require false to start, i use this trick mostly with AND's

Collectives™ on Stack Overflow

How can I secure SQL parameters with entity framework SqlQuery?

2 Answers 2

2 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related