SLD Injection, it's a thing

I'm in the middle of preparing a session about security, and one topic you regularly bump into when thinking about security and writing code is SQL Injection. Although it is the year 2016, there are still people writing code which is vulnerable against SQL Injection.

But I'm not going to focus on SQL Injection here, I'm going to take it one step further. If you want to read up on SQL Injection, Bing/Google is your friend. Or just take a look at good old Bobby Tables.

People often think: "It's called SQL Injection, but I'm not using SQL here, so I'm safe."

Wrong.

From the moment you're using any type of query or filtering language construct, you can introduce some form of SQL Injection in your code:

Using (n)Hibernate with HQL? Are you using parameters in those queries or not?
You're using Azure Table Storage or another NoSQL DB. But does NoSQL mean No SQL Injection?

Another fun example uses the System.Linq.Dynamic NuGet package. This little gem allows you to use string expressions to filter on IEnumerable's. And because it makes our developer life easier, we could use this to filter data in a Web API, for example. Take a look at this piece of code:

public async Task<IEnumerable<User>> GetUsersAsync(string filter)
{
    // Lets say the current user can only access data with odd ID's
    var fullFilter = "(id % 2) = 1";

    if (!string.IsNullOrEmpty(filter))
    {
        fullFilter += $" && {filter}";
    }

    return (await DbContext.Users.ToListAsync()).Where(fullFilter);
}

Yes, I know this is not the best code because you're fetching all users from the database and then you reduce the result set, but bear with me. I'm making a statement here. Besides, this kind of code is still being written in production as well!

As you can see, you're applying a filter to a list of User instances using System.Linq.Dynamic. That filter could be, for example, this:

"name.StartsWith(\"n\")"

This would reduce the results down to all users with an odd ID and where the surname starts with "n". But someone could circumvent our fixed "odd ID" filter part, which would ignore our little security measure, by changing the filter a bit:

1 = 1 || 1 = 1

This filter value would make fullFilter look like this:

(id % 2) = 1 && 1 = 1 || 1 = 1

Because we're missing parentheses around the user filter part, the ID filter will be ignored. One solution could be to add these parentheses.

public async Task<IEnumerable<User>> GetUsersAsync(string filter)
{
    // Lets say the current user can only access data with odd ID's
    var fullFilter = "(id % 2) = 1";

    if (!string.IsNullOrEmpty(filter))
    {
        fullFilter += $" && ({filter})";
    }

    return (await DbContext.Users.ToListAsync()).Where(fullFilter);
}

Actually, you could solve that problem by moving the fixed filter part. That way, no matter what your user is requesting, he or she can't get by the fixed filter anymore:

public async Task<IEnumerable<User>> GetUsersAsync(string filter)
{
   // Lets say the current user can only access data with odd ID's
   var query = DbContext.Users.Where(x => x.Id % 2 == 1);

   if (!string.IsNullOrEmpty(filter))
   {
      return (await query.ToListAsync()).Where(filter);
   }

   return await query.ToListAsync();
}

And to complete the story, you can look at using parameterized queries, which is also supported by System.Linq.Dynamic. Using parameters in your filter would take away some of the filtering power from the consumer of your API, but you could reintroduce that power by defining your own filters. This example shows a basic attempt at implementing this:

public async Task<IEnumerable<User>> GetUsersAsync(string filter)
{
    // Lets say the current user can only access data with odd ID's
    var query = DbContext.Users.Where(x => x.Id % 2 == 1);

    if (!string.IsNullOrEmpty(filter))
    {
        var parsed = ParseFilter(filter);
        return (await query.ToListAsync()).Where(parsed.Filter, parsed.ParamList.ToArray());
    }

    return await query.ToListAsync();
}

private ParsedFilter ParseFilter(string filter)
{
    var newFilter = new StringBuilder(filter.Length);
    var paramList = new List<object>();

    var filterParts = filter.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
    var i = 0;
    var paramIndex = 0;

    while (i < filterParts.Length)
    {
        string @operator = "", property = "";
        object literal = null;
        var parts = new[] {filterParts[i++], filterParts[i++], filterParts[i++]};

        foreach (var part in parts)
        {
            if (IsStringLiteral(part))
            {
                literal = part.Substring(1, part.Length - 2);
                continue;
            }

            if (IsBoolLiteral(part))
            {
                literal = bool.Parse(part);
                continue;
            }

            // No support for float/double/decimal/... yet
            if (IsNumericLiteral(part))
            {
                literal = int.Parse(part);
                continue;
            }

            var propOrOp = part.ToLowerInvariant();
            if (IsOperator(propOrOp))
            {
                @operator = propOrOp;
                continue;
            }

            property = propOrOp;
        }

        newFilter.Append(UseOperator(@operator, property, paramIndex++));
        paramList.Add(literal);

        // We only support "and" and "or". No ! or () allowed.
        if (i < filterParts.Length)
        {
            newFilter.Append(string.Equals(filterParts[i++], "and", StringComparison.OrdinalIgnoreCase) ? " &amp;&amp; " : " || ");
        }
    }

    return new ParsedFilter(newFilter.ToString(), paramList);
}

private bool IsNumericLiteral(string literal)
{
    var rxNumeric = new Regex(@"^-?\d+(,\d+)*(\.\d+(e\d+)?)?$");
    return rxNumeric.IsMatch(literal);
}

private bool IsBoolLiteral(string literal)
{
    return string.Equals(literal, "true", StringComparison.OrdinalIgnoreCase) ||
           string.Equals(literal, "false", StringComparison.OrdinalIgnoreCase);
}

private bool IsStringLiteral(string literal)
{
    return literal.StartsWith("\"", StringComparison.Ordinal) &&
           literal.EndsWith("\"", StringComparison.Ordinal);
}

private bool IsOperator(string @operator)
{
    switch (@operator)
    {
        case "eq":
        case "ne":
        case "lt":
        case "le":
        case "gt":
        case "ge":
        case "sw":
        case "ew":
        case "co":
            return true;
    }

    return false;
}

private string UseOperator(string @operator, string property, int paramIndex)
{
    switch (@operator)
    {
        case "eq":
            return $"{property} = @{paramIndex}";
        case "ne":
            return $"{property} != @{paramIndex}";
        case "lt":
            return $"{property} < @{paramIndex}";
        case "le":
            return $"{property} <= @{paramIndex}";
        case "gt":
            return $"{property} > @{paramIndex}";
        case "ge":
            return $"{property} >= @{paramIndex}";
        case "sw":
            return $"{property}.StartsWith(@{paramIndex})";
        case "ew":
            return $"{property}.EndsWith(@{paramIndex})";
        case "co":
            return $"{property}.Contains(@{paramIndex})";
    }

    return "";
}

private struct ParsedFilter
{
    public ParsedFilter(string filter, IEnumerable<object> paramList)
    {
        Filter = filter;
        ParamList = paramList;
    }

    public string Filter { get; }
    public IEnumerable<object> ParamList { get; }
}

SLD Injection, it's a thing

Comments

More from this blog

Are you sure your access tokens are really secure?

Be wary of account enumeration possibilities

When a session stops working because you're being too strict...

A way to deal with HTTP error responses

Debugging 101

Command Palette

Comments

More from this blog