CODEWITHSUNDEEP
c#sqlasp.netregexsql-injection
castaway2000
castaway2000

Reputation: 314

detectecting destructive SQL queries with C#

So I am looking to find a more effective way to determine all variants of the strings in the array in this this C# code I wrote. I could loop over the whole string and compare each character in sqltext to the one before it and make it overly complicated or i could try to learn something new. I was thinking there has to be a more efficient way. I showed this to a co-worker and she suggested I use a regular expression. I have looked into regular expressions a little bit, but i cant seem to find the right expression.

what I am looking for is a version that takes all variants of the indexes of the array in this code:

public bool securitycheck(String sqltext)
        {
            string[] badSqlList = new string[] {"insert","Insert","INSERT",
                                                "update","Update","UPDATE",
                                                "delete","Delete","DELETE",
                                                "drop","Drop", "DROP"};
            for (int i = 0; i < badSqlList.Count(); i++)
            {
                if (sqltext.Contains(badSqlList[i]) == true)
                {
                    return true;
                }
            }
            return false;
        }

but takes into account for alternate spelling. this code for example does not take into account for "iNsert, UpDate, dELETE, DrOP" but according to my coworker there is a way using Regular expressions to take into account for this.

What is the best way to do this in your opinion?

[Update]

thank you everyone, there is lots of really good information here and it really does open my eyes to handling SQL programatically. the scope on this tool I am building is very small and anyone with the permissions to access this tool and who has intent on being malicious would be someone who has direct access to the database anyway. these checks are in place to more or less prevent laziness. The use-case does not permit for parameterized queries or i would be doing that. your insight has been very educational and I appreciate all your help!

Upvotes: 0

Views: 1375

Answers (2)

Nikola Bogdanović
Nikola Bogdanović

Reputation: 3213

Do not reinvent the wheel - just use parameterized queries as everyone here tells you (fixes even more problem than you are currently aware), you'll thank as all in the future...

But do use this to sanitaze all your filter strings that go in WHERE clauses:

    public static string EscapeSpecial(string s)
    {
        Contract.Requires(s != null);

        var sb = new StringBuilder();
        foreach(char c in s)
        {
            switch(c)
            {
                case '[':
                case ']':
                case '%':
                case '*':
                {
                    sb.AppendFormat(CultureInfo.InvariantCulture, "[{0}]", c);
                    break;
                }
                case '\'':
                {
                    sb.Append("''");
                    break;
                }
                default:
                {
                    sb.Append(c);
                    break;
                }
            }
        }
        return sb.ToString();
    }

Upvotes: 1

Habib
Habib

Reputation: 223257

You can do:

if (badSqlList.Any(r => sqltext.IndexOf(r, StringComparison.InvariantCultureIgnoreCase) >= 0))
{
    //bad SQL found
}

IndexOf with StringComparison enum value will ensure case insensitive comparison.

Another approach could be:

return sqltext.Split()
        .Intersect(badSqlList,StringComparer.InvariantCultureIgnoreCase)
        .Any()

Split your Sql on white space and then compare each word with your white list array. This could save you in cases where your legal table name has keyword like INESRTEDStudents

Not really sure about your requirements, but, generally, a better option would be to use Parameterized queries in the first place. You can't be 100% sure with your white list and there still would be ways to bypass it.

Upvotes: 2

Related Questions