Search code examples
c#asp.netsqlsql-injection

Removing SQL injection in following query in ASP.NET?


I am trying to secure the following SQL query through Parameters, but I am not getting same functionality.

Original SQL query:

SqlDataSource1.SelectCommand = "SELECT ASP_USER.TRANS_MASTER_EMEAPAY.* FROM ASP_USER.TRANS_MASTER_EMEAPAY where " + cond;

After using Parameters,

String strQuery=SELECT ASP_USER.TRANS_MASTER_EMEAPAY.* FROM ASP_USER.TRANS_MASTER_EMEAPAY where @cond;
SqlCommand cmd = new SqlCommand(strQuery);
cmd.Parameters.AddWithValue(@cond, cond);
//blabla...

We are getting the value of cond through function call.

Can anyone suggest how to remove SQL injection in this type of query?
Is there any other way of securing this type of query?

cond is defined as: cond=Condition() and Condition() is defined as follows: We are using cond as follows : cond=Condition(); and Condition function is defined as follows : public String Condition()

{
    string condStr = " 1 = 1";

    if (ddlCountry.SelectedValue != "" && ddlCountry.SelectedValue != "0")
    {

        condStr = condStr + " AND [Country] = '" + ddlCountry.SelectedValue + "'";
    }

    if (txtBatch.Text != "")
    {

        condStr = condStr + " AND [BATCH] = '" + txtBatch.Text + "'";
    }

    if (txtpid.Text != "")
    {

        condStr = condStr + " AND [PARTNER_PRO_ID] = '" + txtpid.Text + "'";
    }

    return condStr;


}

Solution

  • I think you want to achieve sth like this:

    var cond = "A = 'A' AND B LIKE 'C'";
    SqlDataSource1.SelectCommand = "SELECT ASP_USER.TRANS_MASTER_EMEAPAY.* FROM ASP_USER.TRANS_MASTER_EMEAPAY where " + cond;
    

    It's wrong approach. You should rather build something like this:

    SqlDataSource1.SelectCommand = "SELECT ASP_USER.TRANS_MASTER_EMEAPAY.* FROM ASP_USER.TRANS_MASTER_EMEAPAY where A = @aCond AND B LIKE @cCond;";
    SqlCommand cmd = new SqlCommand(strQuery);
    cmd.Parameters.AddWithValue("aCond", "A");
    cmd.Parameters.AddWithValue("cCond ", "C");
    

    It doesn't suffer from SQL injection.

    If you want to use variable amount of where conditions, you should build query via string builder with placeholders and then use parameters. Ex

    var values = new Dictionary<string, string> { { "A", "B"}, { "A", "C"} };
    
    var lastParam = 0;
    
    var builder = new StringBuilder("SELECT ASP_USER.TRANS_MASTER_EMEAPAY.* FROM ASP_USER.TRANS_MASTER_EMEAPAY where ");
    
    var sqlBld = new SqlCommandBuilder();
    
    foreach (var pair in values) 
    {
        lastParam += 1;
        var column = sqlBld.Quote(pair.Key); // if column name is const, you can skip quote
        builder.AppendFormat("{0} = @{1} ", sqlBld, lastParam); // REMEMBER for including OR or AND after clasule.
    
        cmd.Parameters.AddWithValue(lastParam.ToString(), pair.Value);
    }
    

    Waning: code has been written from memory, so it can contain errors :)

    Of course, you don't have to use dictionary - you could built own structure with - for example - operators.