How can I use a temp table and parameters with sp_executesql?

Question

I added quick-and-dirty whole database search using a stored procedure inspired from this SO question. It works fine, but I'm worried about SQL injection. Of course I use an SQL parameter :

 string query = GetUserInput();
 using (SqlConnection con = new SqlConnection(conString))
 {
        using (SqlCommand cmd = new SqlCommand("SearchAllTables", con))
        {
                cmd.CommandType = CommandType.StoredProcedure;

                cmd.Parameters.Add("@SearchStr", SqlDbType.VarChar).Value = query;

                con.Open();
                SqlDataReader reader = cmd.ExecuteReader();

However, since the SP builds a query and execute it, SQL injection is still possible. Relevant part:

CREATE PROC [dbo].[SearchAllTables] 
(
@SearchStr nvarchar(100)
)
AS
BEGIN

CREATE TABLE #Results (TableName nvarchar(370), ColumnName nvarchar(370), ColumnValue nvarchar(3630), TableId int)
--snip boring part

SET @SearchStr2 = QUOTENAME('%' + @SearchStr + '%','''')
INSERT INTO #Results
EXEC
(
    'SELECT ''' + @TableName +''', ''' + @TableName + '.' + @ColumnName + ''', LEFT(CONVERT(varchar(max), ' + @ColumnName + '), 3630), [id]
     FROM ' + @TableName + ' (NOLOCK) ' +
     ' WHERE CONVERT(varchar(max), ' + @ColumnName + ') LIKE ' + @SearchStr2 --This is vulnerable
)

--snip all the END

SELECT TableName, ColumnName, ColumnValue, TableId FROM #Results

I want to change it to use sp_executesql with parameters to prevent SQL injection. I changed it to:

declare @query nvarchar(1000)
set @query= 'SELECT ''' + @TableName +''', ''' + @TableName + '.' + @ColumnName + ''', LEFT(CONVERT(varchar(max), ' 
            + @ColumnName + '), 3630), [id] FROM ' + @TableName + ' (NOLOCK) ' +
            ' WHERE CONVERT(varchar(max), ' + @ColumnName + ') LIKE @term'
INSERT INTO #Results            
EXEC sp_executesql @query, N'@term nvarchar(100)', @term=@SearchStr2;

However I'm not getting any results now. I tried including the "insert into" in the query and using global temp tables but no dice. What can I do to prevent SQL injection and get the results back? Or am I wrong in my approach?

Answer 1

Some things like table names and columns names cannot be sent as parameters in a dynamic query, so they have to be appended. Of course, this is quite messy (and error prone).

Since you are using C#, I advice to take a look into Dynamic Linq library. It provides some extensions that allow string queries to be used in LINQ queries. Other ways to generate dynamic queries are shown here.

Ok, coming back to your initial problem.

1) Dynamic Linq should allow you to easily write queries such as:

// this requires C# 6.0 to use interpolated strings. String.Format can be used instead
someRepository.GetAll.Where($"{col1} = {value1} And {col2} = {value2}");

So, you have dynamic columns and values, but you need dynamic tables. One way is to have a dynamic way of getting a Repository based on a provided type:

// this contains repositories for all types mapped to used tables
public class UnitOfWork : IUnitOfWork
{
    public IRepository<Table1> Table1Repository { get; private set; }
    public IRepository<Table2> Table2Repository { get; private set; }
    // other come here

    // all these are injected
    public UnitOfWork(IDbContext context, IRepository<Table1> table1Repository, IRepository<Table2> table2Repository
    {
        Table1Repository = table1Repository;
        Table2Repository = table2Repository;
        // other initializations
    }

    // typed version
    public IRepository<T> GetRepository<T>()
        where T: class
    {
        Type thisType = this.GetType();
        foreach (var prop in thisType.GetProperties())
        {
            var propType = prop.PropertyType;

            if (!typeof(IRepository).IsAssignableFrom(propType))
                continue;

            var repoType = propType.GenericTypeArguments[0];
            if (repoType == typeof(T))
                return (IRepository<T>) prop.GetValue(this);
        }

        throw new ArgumentException(String.Format("No repository of type {0} found", typeof(T).FullName));
    }

    // dynamic type version (not tested, just written here)
    public IRepository GetRepository(Type type)
        where T: class
    {
        Type thisType = this.GetType();
        foreach (var prop in thisType.GetProperties())
        {
            var propType = prop.PropertyType;

            if (!typeof(IRepository).IsAssignableFrom(propType))
                continue;

            var repoType = propType.GenericTypeArguments[0];
            if (repoType == type)
                return (IRepository) prop.GetValue(this);
        }

        throw new ArgumentException(String.Format("No repository of type {0} found", typeof(T).FullName));
    }
}

To dynamically get a repository, you need to have a mapping between a table name (or your identifier of the table in the filter values and that type). Something like:

var filterTableMap = new Dictionary<String, Type>()
    {
        { "Table1", typeof(Table1Repository) },
        { "Table2", typeof(Table2Repository) },
        // and so on
    };

Your condition would look like the following:

var query = filterTableMap["Table1"].GetAll.Where($"{col1} = {value1}");

However, this is quite tricky if you want to apply multiple conditions at once.

2) An interesting approach is to use reflection:

// this is a slightly changed and not tested version from the source
public static IEnumerable<T> WhereQuery(this IEnumerable<T> source, string columnName, string propertyValue)
{
    return source.Where(m => { 
        return m.GetType().GetProperty(columnName).GetValue(m, null).ToString().Contains(propertyValue); 
    });
}

This should allow to chain where conditions like this:

var query = filterTableMap["Table1"].GetAll.WhereQuery("col1", value1);
if (value2 != null)
    query = query.WhereQuery("col2", value2);

However, I do not think LINQ2SQL can generate SQL for that Where, so the source must be a list of objects. This is a serious problem, if data is not filtered beforehand to reduce the length.

3) Expression trees seems to be the best choice, as pointed out here. Something like:

var param = Expression.Parameter(typeof(String));
var condition =
    Expression.Lambda<Func<String, bool>>(
        Expression.Equal(
            Expression.Property(param, "Col1"),
            Expression.Constant("Value1", typeof(String))
        ),
        param
    ).Compile(); 
// for LINQ to SQL/Entities skip Compile() call

var query = source.Where(condition);

For Contains the solution is more convoluted, as shown here.

The advantages I see in modelling the filtering in C# are:

1. no messy code
2. no SQL injection (but might lead to LINQ injection)
3. if repositories and dependency injection are used, unit testing is facilitated
4. easier maintenance

Disadvantage:

1. more complexity
2. possible performance problems