Catch the Security Flaw(s) #4

Identify as many security issues as you can with this piece of code:-

    1:     [WebMethod] 

    2:     public string GetEmpName(string empid) 

    3:     { 

    4:         SqlConnection con = new SqlConnection("server=.;database=test;uid=sa;pwd=PassW2rd12"); 

    5:         SqlCommand cmd = new SqlCommand("select username from users where id = " + empid, con); 

    6:         con.Open(); 

    7:         string empname = (string)cmd.ExecuteScalar(); 

    8:         con.Close(); 

    9:         return empname; 

   10:     }

How many did you get? Lets go over them one by one:-

1. The SQL connection string uses SQL Authentication to connect to the server. If possible, you should use Windows authentication instead. This eliminates the need to store credentials in the application.

2. If Windows Authentication is not possible, store the connection string in the web.config file and encrypt it using the aspnet_regiis tool.

3. The account used to connect to the SQL server is “sa”, which is a member of the all powerful sysadmin server role. Instead, the account used to connect to the SQL Server must have the least amount of permissions on the server as needed to do the job.

3. There is no validation for the “empid” parameter. If only integers are expected, try to parse it into an integer, otherwise use a regular expression for white list validation.

4. The “empid” parameters is being concatenated to form a SQL statement, which is then executed. This means that the application is vulnerable to SQL Injection. Instead of concatenating user input to create a SQL statement, use parameterized SQL.

5. There is no structured exception handling in this code. As a result, verbose error messages containing managed exceptions will be sent to the end user. Implement structured exception handling and close the connection in a finally block.

This is how the code looks after restructuring:-

    1:  

    2:     [WebMethod]

    3:     public string GetEmpName(string empid)

    4:     {

    5:         int id;

    6:         if (int.TryParse(empid, out id))

    7:         {

    8:             SqlConnection con = new SqlConnection(System.Web.Configuration.WebConfigurationManager.ConnectionStrings["Conn"].ConnectionString);

    9:             SqlCommand cmd = new SqlCommand("select username from users where id = @id", con);

   10:             cmd.Parameters.Add(new SqlParameter("@id", id));

   11:             try

   12:             {

   13:                 con.Open();

   14:                 return (string)cmd.ExecuteScalar();

   15:             }

   16:             catch (Exception exp)

   17:             {

   18:                 LogException(exp);

   19:                 throw new ApplicationException("An unexpected error occured");

   20:             }

   21:             finally

   22:             {

   23:                 if (con != null && con.State == ConnectionState.Open)

   24:                     con.Close();

   25:             }

   26:         }

   27:  

   28:         return null;

   29:     }