Avoid sql injection with calling function

Question

I try to call function to select data from database,coz it will more efficient and i don't like to open connection and execute reader every time,have any solution can do like that?

this is my first method to select data from database,but will hit sql injection problem

    protected void Button1_Click(object sender, EventArgs e)
    {
        Class1 myClass = new Class1();
        lblAns.Text = myClass.getdata("Table1", "Student", "Student = '" + TextBox1.Text + "'");           
    }


    public string getdata(string table,string field,string condition)
    {
        SqlDataReader rdr;
        SqlConnection conn = new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=|DataDirectory|\Database1.mdf;Integrated Security=True;User Instance=True");
        string sql = "select " + field + " from " + table + " where " + condition;

        try
        {
            conn.Open();
            SqlCommand cmd = new SqlCommand(sql, conn);
            rdr = cmd.ExecuteReader();
            while (rdr.Read())
            {
                return "true";
            }
        }
        catch (System.Data.SqlClient.SqlException ex)
        {
            string msg = "Insert Error:";
            msg += ex.Message;
        }
        finally
        {
            conn.Close();
        }
        return "false";
    }

this is my second method but will hit error (ExecuteReader requires an open and available Connection. The connection's current state is closed.) at line (rdr = cmd.ExecuteReader();)

     public string getdata(SqlCommand command,SqlConnection conn)
    {
        SqlDataReader rdr;
        try
        {
            conn.Open();
            SqlCommand cmd = new SqlCommand();
            cmd = command;
            rdr = cmd.ExecuteReader();
            while (rdr.Read())
            {
                return "true";
            }
        }
        catch (System.Data.SqlClient.SqlException ex)
        {
            string msg = "Select Error:";
            msg += ex.Message;
        }
        finally
        {
            conn.Close();
        }
        return "false";
    }

    public SqlConnection conn()
    {
        SqlConnection conn =  new SqlConnection(@"Data Source=.\SQLEXPRESS;AttachDbFilename=|DataDirectory|\Database1.mdf;Integrated Security=True;User Instance=True");
        return conn;
    }

    protected void Button1_Click(object sender, EventArgs e)
    {
        Class1 myClass = new Class1();
        string strSql;

        strSql = "Select student from Table1 where student=@stu";
        SqlCommand command = new SqlCommand(strSql, myClass.conn());
        command.Parameters.AddWithValue("@stu", TextBox1.Text);
        myClass.getdata(command, myClass.conn());
    }

have solution can use 1st method but will not hit the sql injection problem?

What's your problem with the second solution? Use it if it works, it's much safer. And why do you create a new SQL Connection all the time? Simply create it once at program start and access it via getter. — jAC
– jAC, Commented May 16, 2013 at 8:15
second solution is way more secure (would be even better calling a stored procedure or a user defined function). If you don't want to manipulate connections, readers and commands in your code, next step will be to use an ORM (Entity Framework, NHibernate, ...) — jbl
– jbl, Commented May 16, 2013 at 8:19
2nd solution hit this problem - (ExecuteReader requires an open and available Connection. The connection's current state is closed.) at line (rdr = cmd.ExecuteReader();) — user1848681
– user1848681, Commented May 16, 2013 at 8:20
Then simply open then connection with conn.Open(); when this exception is thrown and execute the reader again. — jAC
– jAC, Commented May 16, 2013 at 8:29

Luke Girvin · Accepted Answer · 2013-05-16 08:27:22Z

Use ALWAYS the second solution. The only way to avoid Sql Injection is through the use of parameterized queries.

Also fix the error on the second example. You don't associate the connection to the command, also it is a bad practice to keep a global object for the connection. In ADO.NET exist the concept of Connection Pooling that avoid the costly open/close of the connection while maintaining a safe Handling of these objects

public string getdata(SqlCommand command)
{
    // Using statement to be sure to dispose the connection
    using(SqlConnection conn = new SqlConnection(connectionString))
    {
       try
       {
            conn.Open();
            cmd.Connection = conn;
            SqlDataReader rdr = cmd.ExecuteReader();
            while (rdr.Read())
            {
                return "true";
            }
        }
        catch (System.Data.SqlClient.SqlException ex)
        {
             string msg = "Select Error:";
             msg += ex.Message;
             return msg;
        }
  }
  return "false";
}

Collectives™ on Stack Overflow

Avoid sql injection with calling function

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related