CODEWITHSUNDEEP
sqlsql-serverc#-4.0sql-injection
user3441151
user3441151

Reputation: 1910

Safety from SQL injection

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.

Upvotes: 0

Views: 117

Answers (3)

Md.Rajibul Ahsan
Md.Rajibul Ahsan

Reputation: 853

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();

Upvotes: 0

Croc Studio
Croc Studio

Reputation: 141

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.

Upvotes: 1

MDiesel
MDiesel

Reputation: 2667

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).

Upvotes: 1

Related Questions