Question

SqlConnection con = new SqlConnection(ConfigurationManager.ConnectionStrings["techconn"].ToString());

            SqlCommand com = new SqlCommand("select * from hs where ac between'" + TextBox1.Text + "'and '" + TextBox2.Text + "' and em='" + DropDownList1.SelectedItem.Text.ToString() + "'", con);

            DataTable dt = new DataTable();

            con.Open();

            SqlDataAdapter sqlDa = new SqlDataAdapter(com);

            sqlDa.Fill(dt);

            if (dt.Rows.Count > 0)
            {
                GridView1.DataSource = dt;
                GridView1.DataBind();
            }
            else
            {
                GridView1.Visible = false;
            }

            con.Close();

Is this code safe from SQL injection?

If not, please correct this code that it is safe from SQL injection.

I am using SQL Server 2008.

Was it helpful?

Solution

In short, the answer is no. You need to always use parameters in your queries.

SqlCommand com = new SqlCommand("select * from hs where ac between @ac1 and @ac2 and em=@em", con);

You then add the parameters to your SqlCommand object (com).

OTHER TIPS

Yes your code is quite prone to issues, not only sql injection attacks. Try the following:

    public DataTable GetData(string textbox1, string textbox2, string dropdown)
    {
        DataTable result = null;
        string connString = null;

        if (ConfigurationManager.ConnectionStrings["techconn"] != null)
            connString = ConfigurationManager.ConnectionStrings["techconn"].ConnectionString;

        if (!string.IsNullOrEmpty(connString))
        using (SqlConnection con = new SqlConnection(connString))
        {
            con.Open();

            using (SqlCommand cmd = con.CreateCommand())
            {
                cmd.CommandText = "select * from hs where (ac between @a and @b) and em = @c";

                cmd.Parameters.AddWithValue("@a", textbox1);
                cmd.Parameters.AddWithValue("@b", textbox2);
                cmd.Parameters.AddWithValue("@c", dropdown);

                using (SqlDataAdapter da = new SqlDataAdapter(cmd))
                {
                    result = new DataTable();
                    da.Fill(result);

                }
            }
        }

        return result;

    }

Paste it in your code and use by

DataTable dt = GetData(TextBox1.Text, TextBox2.Text, DropDownList1.SelectedItem.Text.ToString());

            if (dt != null && dt.Rows.Count > 0)
            {
                GridView1.DataSource = dt;
                GridView1.DataBind();
            }
            else
            {
                GridView1.Visible = false;
            }

Test it properly too.

IF you use direct textbox reference to your sql query then it will never be SQL injection safe any end user can pass Injecting values to your text box and it will be injected.

Never use UI elements directly to your SQL you can try the below line of CODE

SqlConnection conn = new SqlConnection(_connectionString);  
conn.Open();  
string s = "select * from hs where ac between @TextBoxOnevaluevariable and
@TextBoxTwovaluevariable and em=@DropdownSelectedTextVariable";

SqlCommand cmd = new SqlCommand(s);  
cmd.Parameters.Add("@TextBoxOnevaluevariable", Texbox1.Text);  
cmd.Parameters.Add("@TextBoxTwovaluevariable", Texbox2.Text);  
cmd.Parameters.Add("@DropdownSelectedTextVariable",DropDownList1.SelectedItem.Text.ToString());  
SqlDataReader reader = cmd.ExecuteReader();
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top