0

I would like to use Linq instead of below hardcoded Sql Injection to search from SqlServer DATABASE TABLES. How to retrieve Dynamically generated web controls input texts in C# Linq and replace the entire Sql injection in Linq for searching.

My C# code:

protected void Search_Button_Click(object sender, EventArgs e) 
{
    try 
     {  
       Table maintable = Select.FindControl("dynamic_filter_table_id") as Table;
       int rc = maintable.Rows.Count;
            if (rc == 1)
            {

                DropDownList D1 = maintable.FindControl("MainDDL") as DropDownList;
                if (D1.SelectedValue.Contains("decimal"))
                {
                    TextBox T1 = maintable.FindControl("txtbox1") as TextBox;
                    TextBox T2 = maintable.FindControl("txtbox2") as TextBox;
                    SqlDataAdapter sql = new SqlDataAdapter("SELECT F.Col1,F.Col2,V.COL1, col2,col3, col4 , col5, cl6 FROM TABLE1 as V , TABL2 as F WHERE V.Col1 = F.Col1 AND " + DDL1.SelectedItem.Text + " >= " + T1.Text + " AND " + DDl1.SelectedItem.Text + " <= " + T2.Text, con);

                    DataSet data = new DataSet();
                    sql.Fill(data);
                    con.Close();
                    Session["DataforSearch_DDL"] = data.Tables[0];
                }
            }
       }

    catch
    {
      ImproperSearch();
    }
}
Shrivatsan
  • 105
  • 1
  • 18
  • Changing all Queries into LINQ is time consuming and ultimately depends on size of your project. To Repair code to save from sql injection, Better option is, use parameterised queries. refer this two links 1) http://www.aspsnippets.com/Articles/Using-Parameterized-queries-to-prevent-SQL-Injection-Attacks-in-SQL-Server.aspx 2) http://stackoverflow.com/questions/5468425/how-do-parameterized-queries-help-against-sql-injection – Sandip May 30 '14 at 05:24

2 Answers2

4

Why don't you just rewrite your query to be SQL-injection safe? LINQ won't give you any benefit.

You can achieve that by doing two things.

The first is to secure the column names. That is accomplished by specifying which characters is allowed for column names (more secure than trying to figure out what characters is not allowed). In this case I remove everything but letters and digits. If you have column names which contains underscore, just add that to the check.

The next thing is to use parameterized queries. Each ADO.NET driver has built in support for that, so you just have to specify the value using cmd.Parameters.AddWithValue. By doing so the value isn't part of the query string and hence there is no potential SQL injection.

using (var con = yourConnectionFactory.Create())
{
    using (var cmd = new SqlCommand(con))
    {
        var safeKey1 = OnlyLettersAndDigits(DDL1.SelectedItem.Text);
        var safeKey2 = OnlyLettersAndDigits(DDL2.SelectedItem.Text);

        cmd.CommandText = "SELECT F.Col1,F.Col2,V.COL1, col2,col3, col4 , col5, cl6 " + 
                              " FROM TABLE1 as V , TABL2 as F WHERE V.Col1 = F.Col1 " +
                              " AND " + safeKey1 + " >= @text1 " + 
                              " AND " + safeKey2 + " <= @text2 ";
            cmd.Parameters.AddWithValue("text1", T1.Text);
            cmd.Parameters.AddWithValue("text2", T2.Text); 
            var adapter = new SqlDataAdapter(cmd);

            var data = new DataSet();
            sql.Fill(data);
            Session["DataforSearch_DDL"] = data.Tables[0];
    }
}

public string OnlyLettersAndDigits(string value)
{
    var stripped = "";
    foreach (var ch in value)
    {
        if (char.IsLetterOrDigit(ch))
            stripped += ch;
    }

    return stripped;
}
jgauffin
  • 99,844
  • 45
  • 235
  • 372
  • "LINQ won't give you any benefit" can't disagree more. Inherently you can't SQL inject with Linq (baring bugs in your code), however it only takes a single mistake to open up a SQL injection hole. The reason 'why not' fix all your queries to be parameterized, is that it is very hard to know where you have potential for SQL injection in ADO.net – Aron May 30 '14 at 06:31
  • @Aron: ***a)*** Are you seriously recommending him to convert what looks like legacy code to EF just to achieve SQL injection safety? ***b)*** What's hard to know? Never append anything that the user have entered into the query string. It's a very simple rule. ***c)*** his code would require a conversion from column names in a string variable to LINQ expressions which would be a lot more complex than the current solution. – jgauffin May 30 '14 at 06:33
  • There are some good micro-ORMs on the market, some of them rely on attributes for the SQL. Given that attribute constructor parameters must be const, there is much less scope for #$%&ing up. Currently drafting my answer. My point is if every programmer followed every "simple" rule, half the bugs in the world wouldn't exist. – Aron May 30 '14 at 06:38
  • @Aron: I know of micro orms, I've written one myself where I intentionally avoided LINQ: http://blog.gauffin.org/2014/02/introducing-the-data-mapper-in-griffin-framework/. Still. imho it's never a good idea to rewrite code just to achieve such a simple thing. – jgauffin May 30 '14 at 06:40
  • we are talking about a cross cutting concern. You either rewrite using shotgun programming (and risk missing a target in your surface area) or you rewrite it using a method which addresses your cross cutting concern. Either way you have to rewrite it! – Aron May 30 '14 at 06:44
  • Actually...re-reading the question, this is actually a case where the query HAS to explicitely allow SQL injection, for the columns. Only way I know to do this "safely" is with EF Expression Trees. – Aron May 30 '14 at 06:49
  • Im my experience legacy code projects can be quite large, and you don't want to introduce new libraries/frameworks without careful thought. It can produce worse code. He asked for a solution for a specific problem and typically that's what we should give him unless he asks for something else. – jgauffin May 30 '14 at 06:51
  • EF expression trees will be a bloated solution. Look at his query, it's very easy to make the column names safe using a simple function. – jgauffin May 30 '14 at 06:53
  • 1
    You are of course free to add an alternative solution. I see no point in further discussion as we seem to have very different views on the subject :) – jgauffin May 30 '14 at 06:54
2

You can use store procedure for customer search, It will work for both ADO.Net and LINQ approach. Just create a SP and add it to your DBML file very simple.

Here is the link how you can use SP in LINQ

http://www.mssqltips.com/sqlservertip/1542/using-stored-procedures-with-linq-to-sql/

cachet.net
  • 310
  • 3
  • 16