SQL injection prevention with 'AS QUERY'

I have the following code which seems pretty standard on face value, however in query is another SQL statement hence why the 'AS QUERY' is at the end of the SQL string. I wanted to know if there was a sophisticated approach to parameterising the following SQL command instead of concatenating the entire query together.

The only solution I could think of would be to instead of having a query as a string, have it as an SQLCommand type object and initiate 2 commands. 1 to could and the other to display the preview of the data.

public static CommandStatus<int> GetQueryRecordCount(SqlConnection connection, String query)
{
    String sql = "SELECT COUNT(1) FROM (" + query + ") AS QUERY";
    SqlCommand cmd = new SqlCommand();
    cmd.CommandType = CommandType.Text;
    cmd.CommandText = sql;
    cmd.Connection = connection;
    cmd.CommandTimeout = GetTimeout();

    try
    {
        SqlDataReader dataReader = cmd.ExecuteReader();
        dataReader.Read();

        String count = dataReader[0].ToString();
        dataReader.Close();

        return new CommandStatus<int>(Int32.Parse(count));
    }
    catch (Exception e)
    {
        return new CommandStatus<int>("Failed to GetQueryRecordCount[" + sql + "]:" + e.Message, e);
    }
}

String SQL will end up being something like this

"SELECT COUNT(1) FROM (SELECT TOP 20 [RecordID],[Name],[SonsName],[DadsName],[MothersName],[DaughtersName] FROM [dbo].[sample] ) AS QUERY"

1 answer

  • answered 2019-10-24 16:19 Bill Karwin

    This function is literally SQL injection by design.

    Whitelisting the SQL queries this function will accept is the only way to make it safe.

    That is, the caller won't be able to inject any SQL query, they'll only be able to pick from a fixed list of pre-vetted queries. The list could even be defined as an array of static strings in the function you show.

    But then they don't need to pass the whole query as a string, they only need to pass an ordinal integer to identify which query in the whitelist to run.