-3

After R&D I didn't get solution for this flaw. Please guide me in solving this flaw

Description
This database query contains a SQL injection flaw. The function call constructs a dynamic SQL query using a variable derived from user-supplied input. An attacker could exploit this flaw to execute arbitrary SQL queries against the database.

Recommendations
Avoid dynamically constructing SQL queries. Instead, use parameterized prepared statements to prevent the database from interpreting the contents of bind variables as part of the query. Always validate user-supplied input to ensure that it conforms to the expected format, using centralized data validation routines when possible.

public void EditUser(User f)
{
    string connectionString 
        = ConfigurationSettings.AppSettings["ConnectionString"];

    SqlConnection conn = new SqlConnection(connectionString);

    SqlCommand cmd = conn.CreateCommand();
    string adm = (f.IsAdmin?"1":"0");
    string phone="",email="";

    if(f.PhoneList.Count > 0)
       phone = f.PhoneList[0].ToString();

    if(f.EmailList.Count > 0)
       email = f.EmailList[0].ToString();

    for(int i = 1; i < f.PhoneList.Count; i++)
       phone = phone + ";" + f.PhoneList[i].ToString();

    for(int i = 1; i < f.EmailList.Count; i++) 
       email = email + ";" + f.EmailList[i].ToString();

    cmd.CommandText = "UPDATE Users SET  is_admin="+adm+",login='"+f.Login+"',passwd='"+f.Password+
             "',firstname='"+f.FirstName+"',lastname='"+f.LastName+"',email='"+email+"',phone='"+phone+"',address='"+f.Address+"' where user_id="+f.UserId;

    conn.Open();
    int affected = cmd.ExecuteNonQuery();
    cmd.ExecuteNonQuery();
}
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Kiran SK
  • 9
  • 1
  • 2
  • 6

1 Answers1

6

Your code builds up the SQL command by appending strings together

    cmd.CommandText 
        = "UPDATE Users SET  is_admin="+adm+",login='"+f.Login+"',passwd='"+f.Password+
         "',firstname='"+f.FirstName+"',lastname='"+f.LastName+"',email='"+email+"',phone='"+phone+"',address='"+f.Address+"' where user_id="+f.UserId;

Suppose someone called your function with a User value where lastname equaled

"Presser';DROP TABLE Users --"

Your SQL command will now destroy the Users table.

See here, where MSDN describes how to avoid this problem.

Ross Presser
  • 6,027
  • 1
  • 34
  • 66