Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is there a SQL Injection risk with this query? If so, how can I avoid it?

I usually create parameterized queries in order to avoid SQL Injection attacks. However, I have this particular situation where I haven't been totally able to do it:

public DataSet getLiveAccountingDSByParameterAndValue(string parameter, string value)
{
    string sql = "select table_ref as Source, method as Method, sip_code as Code, " +
        " from view_accountandmissed " +
        " where " + parameter + " like @value " +
        " order by time DESC ";
    MySqlCommand cmd = commonDA.createCommand(sql);
    cmd.Parameters.Add("@value", MySqlDbType.String);
    cmd.Parameters["@value"].Value = "%" + value + "%";

    MySqlDataAdapter objDA = commonDA.createDataAdapter(cmd);
    DataSet objDS = new DataSet();
    objDA.Fill(objDS);
    return objDS;
}

As you can see, I am creating @value as a parameter but if I tried to do the same with parameter the query would fail.

So, is there a risk of SQL Injection with this query? Also, take into account that parameter is set by a DropDownList's SelectedValue (not a TextBox, so the input is limited). If so, how can I improve this query?

like image 683
aleafonso Avatar asked Feb 21 '23 13:02

aleafonso


2 Answers

Yes there is:

" where " + parameter + " like @value " +

The value in parameter is your risk. In the postback you should check if the selected value is in the set of start values of the dropdown list.

Make the parameter an enum and pass the enum to your function. That will eliminate the risk (something like: not tested):

public DataSet getLiveAccountingDSByParameterAndValue(ParameterEnum parameter, string value)
.....
    " where " + parameter.ToString() + " like @value " +

The ParameterEnum contains a list of all possible values in your dropdown list. In your code behind, parse the selected value to the enum.

like image 116
Peter Avatar answered Apr 19 '23 23:04

Peter


So, is there a risk of SQL Injection with this query?

I think yes, it's vulnerable to SQL injection. For example, parameter = "1=1 OR value"

Also, take into account that parameter is set by a DropDownList's SelectedValue (not a TextBox, so the input is limited)

Doesn't really matter. A malicious user can inject any value on the executable itself or on the network packet (and thus send a value that doesn't exist on the DropDown).

If so, how can I improve this query?

You should check parameter argument and compare with DropDown values. For more generic data, I think there should be libraries that check such things (but I have no C# idea...).

like image 40
m0skit0 Avatar answered Apr 19 '23 23:04

m0skit0