0

I have been working on a project and I am very new to C#. I have Login which is correct but it is vulnerable to SQL injection attacks. Here is my code - can anybody help me out how to apply a stored procedure with parameters so it can be more secure?

protected void Button1_Click(object sender, EventArgs e)
{
    string Cs = ConfigurationManager.ConnectionStrings["MyDatabase1ConnectionString"].ConnectionString;

    using(SqlConnection con=new SqlConnection(Cs))
    {
        SqlCommand cmd = new SqlCommand("Select * from Users where Username= '" + Username.Text + "' And " +
                    "Password='" + Password.Text+ "'", con);

        con.Open();

        SqlDataAdapter sda = new SqlDataAdapter(cmd);

        DataTable dt = new DataTable();
        sda.Fill(dt);

        if (dt.Rows.Count != 0)
        {
            Response.Redirect("~/Cuhome.aspx");
        }
        else
        {
            LblError.Text = "Invalid Username & Password";
        }
    }
}

Thanks

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
  • I couldn't understand the concept of how to put together. i really appreciated your help. – yasmin ahmed Aug 04 '18 at 07:40
  • Did you try researching on Google? Or you want us to do that for you? https://learn.microsoft.com/en-us/sql/relational-databases/stored-procedures/create-a-stored-procedure?view=sql-server-2017 – Chetan Aug 04 '18 at 07:43
  • https://stackoverflow.com/questions/7542517/call-a-stored-procedure-with-parameter-in-c-sharp – Chetan Aug 04 '18 at 07:43
  • 1
    https://visualstudiomagazine.com/articles/2017/07/01/parameterized-queries.aspx?m=1 – Chetan Aug 04 '18 at 07:45
  • 1
    It looks like you are creating an asp.net project. If you create an MVC project instead, you get all of this functionality, I'm a standard and highly secure way, for free. In VS create a new solution and choose mvc. – Neil Aug 04 '18 at 08:12
  • @Neil is correct - you should not be building this yourself. – mjwills Aug 04 '18 at 08:39

1 Answers1

3

You don't necessarily need a stored procedure - just use a properly parametrized query - that achieves the same goal of being "more secure":

protected void Button1_Click(object sender, EventArgs e)
{
    string Cs = ConfigurationManager.ConnectionStrings["MyDatabase1ConnectionString"].ConnectionString;

    // set up query - using PARAMETERS as you always should!
    // Also: you don't seem to need the *whole* row - all the columns of "Users" - so select just what you **really need**!
    string query = "Select UserId from Users where username = @username and password = @password;";

    // put both SqlConnection *AND* SqlCommand into "using" blocks
    using (SqlConnection con=new SqlConnection(Cs))
    using (SqlCommand cmd = new SqlCommand(query, con))
    {
        // provide the parameters
        cmd.Parameters.Add("@username", SqlDbType.VarChar, 100).Value = Username.Text;
        cmd.Parameters.Add("@password", SqlDbType.VarChar, 100).Value = Password.Text;

        // use an ExecuteScalar call to get the UserId from Users - and check if it exists
        con.Open();

        object result = cmd.ExecuteScalar();

        // if we get something back --> the user with this password exists --> redirect
        if (result != null)
        {
            Response.Redirect("~/Cuhome.aspx");
        }
        else
        {
            LblError.Text = "Invalid Username & Password";
        }
    }
}

But this code has a second, even more horribly flaw: you seem to be storing the password for your users in PLAIN TEXT in your database table! That's a MAJOR no-no for any secure site - do NOT EVER store passwords in plain text !! You need to hash and salt password, if you actually store them.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
  • 1
    Adding that in addition to better security, parameterized queries eliminate the need to enclose literals (or not) depending on the type, eliminate the need to escape quotes within strings, avoid the need to format date string literals in a particular way, do not require decimal separators, improve performance by promoting execution plan cache reuse, and result in cleaner code that's easier to maintain. – Dan Guzman Aug 04 '18 at 10:28
  • 1
    +1 for strongly-typed parameters instead of [AddWithValue](https://www.dbdelta.com/addwithvalue-is-evil/). – Dan Guzman Aug 04 '18 at 10:30
  • I really appreciate your help i tried to do this but couldn't figure out. Thank You so much. – yasmin ahmed Aug 04 '18 at 17:05