Pregunta

I'm working on C# project and I'm new to this technology.

I want to read some data from SQL Server 2008, and I write the following code

public User select(string username, string password)
{
    string connection = ConfigurationManager.ConnectionStrings["lawyersDBConnectionString"].ConnectionString.ToString();
    string sql = string.Format("select * from users where userName = '{0}' and password = '{1}'", username, password);

    SqlConnection con = new SqlConnection();            
    con.ConnectionString = connection;

    DataSet ds = new DataSet();
    SqlDataAdapter da = new SqlDataAdapter(sql, con);            

    User user = new User();
    DataRow dr;

    try
    {
            da.Fill(ds);
            dr = ds.Tables[0].Rows[0];

            user.Id = Convert.ToInt16(dr["userID"]);                
            user.FirstName = (string)dr["firstName"];
            user.LastName = (string)dr["lastName"];
            user.Email = (string)dr["email"];
            user.Username = (string)dr["userName"];
            user.Password = (string)dr["password"];
            user.type = (string)dr["type"];

            return user;
    }
    catch (Exception ex)
    {                
            return null;
    }
}//end of select method

But I had read an article about SQL injection, and I want to use SQL parameters to avoid this, but I don't know how.

¿Fue útil?

Solución

This is a simple rework on your code. Not tested, but essentially it consist in adding the using statement around the disposable objects and the use of a SqlCommand with its parameters collection

string connection = ConfigurationManager.ConnectionStrings ["lawyersDBConnectionString"].ConnectionString.ToString();
string sql = "select * from users where userName = @uname and password = @pwd";

 DataSet ds = new DataSet();
 using(SqlConnection con = new SqlConnection(connection))
 using(SqlCommand cmd = new SqlCommand(sql, con))
 {
    con.Open();
    cmd.Parameters.AddWithValue("@uname", username);
    cmd.Parameters.AddWithValue("@pwd", password);

    using(SqlDataAdapter da = new SqlDataAdapter(cmd))
    {
         User user = new User();
         DataRow dr;
         da.Fill(ds);
         dr = ds.Tables[0].Rows[0];

         user.Id = Convert.ToInt16(dr["userID"]);                
         user.FirstName = (string)dr["firstName"];
         user.LastName = (string)dr["lastName"];
         user.Email = (string)dr["email"];
         user.Username = (string)dr["userName"];
         user.Password = (string)dr["password"];
         user.type = (string)dr["type"];
         return user;
    }
}

Notice how the command text doesn't contain directly the strings for user and password but a simple parameter placeholder (@uname and @pwd). These placeholders are referred as the parameters name when adding the parameters to the SqlCommand collection.

Looking at the usage of the data retrieved I strongly suggest you to look at simple ORM tools like Dapper that could directly translate all of this code in the User object

Otros consejos

Interestingly enough, the way String.Format works isn't much different from SQL parameters. The only real difference is that you specify the type of data each parameter is which allows the SQLCommand to properly sanitize (read: prevent sql injection) your user's input.

Here's an example of how you might alter your code to use SQL Parameters.

using (SqlConnection connection = new SqlConnection(connectionString))
{
    connection.Open();
    using (SqlCommand command = new SqlCommand("select * from users where userName = @pUsername and password = @pPassword", connection))
    {
        command.Parameters.Add(new SqlParameter("pUsername", username));
        command.Parameters.Add(new SqlParameter("pPassword", password));

        DataSet ds = new DataSet();
        SqlDataAdapter da = new SqlDataAdapter(command);  

        // The rest of your code here...
     }
}

A few things I'd like to point out though:

  1. Usernames are typically case insensitive so the query should probably use a LIKE or UCASE() comparison to look for the username.
  2. It's apparent from the query that your password is not hashed or salted. This is very bad. Read up on hashing passwords. https://crackstation.net/hashing-security.htm
  3. Basically what you're creating here is called an object relational manager. Unless you're specifically interested in learning how to develop one I highly recommend you use one that's been tried and tested. Personally I use nHibernate. Hibernate was written as an ORM for Java and nHibernate is extremely popular for .Net applications. Entity Framework is Microsoft's ORM. I don't think it's quite on par with nHibernate yet but it's constantly improving.

Here is a reusable method I wrote for that need:

public static DataSet GetDataSetWithParameters(string query, List<SqlParameter> parameters)
{
    DataSet ds = new DataSet();
    SqlConnection Con = new SqlConnection(ConnectionString);
    Con.Open();

    try
    {
        using (SqlCommand cmd = new SqlCommand(query, Con))
        {
            if (parameters != null)
            {
                cmd.Parameters.AddRange(parameters.ToArray());
            }

            using (SqlDataAdapter Adapter = new SqlDataAdapter(cmd))
            {
                Adapter.Fill(ds);
            }

            return ds;
        }
    }
    catch
    {
        throw;
    }
    finally
    {
        CloseConnection(ref Con);
    }
}
Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top