0

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!

13
  • 4
    If you convert the input text to lower case you have very few keywords to check and no problem with spelling permutations Commented Mar 9, 2015 at 18:43
  • 4
    Why would you want to do this at all? Just use parameterized queries instead and the issue is gone. Commented Mar 9, 2015 at 18:43
  • 1
    I'd go with parameterized queries...but if you have to take this approach, you missed the truncate statement. Commented Mar 9, 2015 at 18:49
  • 4
    It's not possible to completely blacklist queries that manipulate data. For example, a user could write a query such as DECLARE @sql varchar(1000); SET @sql = 'TRUN' + 'CATE TABLE Foo'; EXEC (@sql); to bypass any blacklist you come up with. Restricted permissions for the account used to access SQL server would be the only fail-safe way to prevent unauthorized action. Commented Mar 9, 2015 at 18:59
  • 1
    Regex isn't really going to do much more for you here either. Even if you got it to catch all your blacklist words, it's going to reject legitimate queries like SELECT * FROM Report WHERE ReportTitle LIKE '%March Data Insert%', and let through potentially malicious queries like @John Bledsoe's Commented Mar 9, 2015 at 19:04

2 Answers 2

2

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.

Sign up to request clarification or add additional context in comments.

1 Comment

Thank you! I will try this when I get home tonight. unfortunately the use case for this tool accepts lots and lots of various types of queried information and parameters would restrict the tools use. the intended scope of the tool is very specific to managerial administrators so destructive queries are not a huge concern but its still a concern. Thanks for this code snippet.
1

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();
    }

1 Comment

thank you for this. I will play with this code tonight.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.