5

i uwant to cach input, which seems to be like SQL injection. So I wrote the method:

        public static bool IsInjection(string inputText)
    {


        bool isInj = false;


        string regexForTypicalInj = @"/\w*((\%27)|(\'))((\%6F)|o|(\%4F))((\%72)|r|(\%52))/ix";
        Regex reT = new Regex(regexForTypicalInj);
        if (reT.IsMatch(inputText))
            isInj = true;


        string regexForUnion = @"/((\%27)|(\'))union/ix";
        Regex reUn = new Regex(regexForUnion);
        if (reUn.IsMatch(inputText))
            isInj = true;



        string regexForSelect = @"/((\%27)|(\'))select/ix";
        Regex reS = new Regex(regexForSelect);
        if (reS.IsMatch(inputText))
            isInj = true;

        string regexForInsert = @"/((\%27)|(\'))insert/ix";
        Regex reI = new Regex(regexForInsert);
        if (reI.IsMatch(inputText))
            isInj = true;

        string regexForUpdate = @"/((\%27)|(\'))update/ix";
        Regex reU = new Regex(regexForUpdate);
        if (reU.IsMatch(inputText))
            isInj = true;

        string regexForDelete = @"/((\%27)|(\'))delete/ix";
        Regex reDel = new Regex(regexForDelete);
        if (reDel.IsMatch(inputText))
            isInj = true;

        string regexForDrop = @"/((\%27)|(\'))drop/ix";
        Regex reDr = new Regex(regexForDrop);
        if (reDr.IsMatch(inputText))
            isInj = true;

        string regexForAlter = @"/((\%27)|(\'))alter/ix";
        Regex reA = new Regex(regexForAlter);
        if (reA.IsMatch(inputText))
            isInj = true;

        string regexForCreate = @"/((\%27)|(\'))create/ix";
        Regex reC = new Regex(regexForCreate);
        if (reC.IsMatch(inputText))
            isInj = true;

        return isInj;

    }

But seems I have done some mistakes, becouse my code do not detects injections. What i do wrong? I guess theres something wrong in defining Regex expressions?

vts123
  • 1,736
  • 6
  • 27
  • 41
  • 1
    What's your reason for not using parameterized queries in the first place? – JasonTrue Feb 24 '10 at 21:09
  • 1
    Exactly. Parameterized queries = sanitized. Playing with regexes to do the job of a parser = madness. – JasonTrue Feb 24 '10 at 21:19
  • I do this for investigative (sciental) purposes. My framework later would be used to develop win forms applications. Other developers they are beginers often and sometimes do not use Parameters. So first i need somehow to do protection in C# code. – vts123 Feb 24 '10 at 21:25
  • @Vytas999 - And there is now way that you could use parameters under the hood in your framework? – Oded Feb 24 '10 at 21:30
  • It seems to me that detecting that someone is using string concatenation to build SQL queries and reporting that as a possible security risk in static code analysis is more useful than trying to work around such errors. – JasonTrue Feb 24 '10 at 22:54

4 Answers4

15

Don't try to do this with RegEx - there are too many ways around it. See this classic SO answer about parsing with RegEx - it is specific to HTML, but still applies.

You should use Parameters, these are in the BCL and have anti SQL injection measures built in.

Update: (following comments)

If you really must parse the SQL, do not use RegEx for the reasons outlined in the linked article. RegEx is not a parser and should not be used as one.

Use a SQL parser - this should help with sanitizing attempts. Here is one, here another.

You can continue your scientific investigation with these.

Community
  • 1
  • 1
Oded
  • 489,969
  • 99
  • 883
  • 1,009
  • I do this for investigative (sciental) purposes. My framework later would be used to develop win forms applications. Other developers they are beginers often and sometimes do not use Parameters. So first i need somehow to do protection in C# code. – vts123 Feb 24 '10 at 21:27
  • @Vytas999 - I have updated my answer, following your comment. – Oded Feb 24 '10 at 21:42
3

Do not use string parsing or regular expressions to handle this sort of thing. SQL syntax is too complicated to reliably parse with regular expressions.

Instead use parametrized queries with placeholders and avoid string concatenation altogether. This will defeat SQL injection at its root.

var command = new SqlCommand(connection);
command.Text = "INSERT INTO foo (a, b, c) VALUES (@a, @b, @c)";
command.Parameters.AddWithValue("a", "this is invulnerable");
command.Parameters.AddWithValue("b", "to any sort of SQL injection");
command.Parameters.AddWithValue("c", "--'; DROP DATABASE");
command.ExecuteNonQuery();
Alex J
  • 9,905
  • 6
  • 36
  • 46
2

If you really want to help out your "not so experienced programmers", you'd be better off trying to detect when they are doing inline sql in their code. It shouldn't be too difficult to write an FxCop rule to spot it. If you include it as part of a post build process, or if you have team system, set the rule to fail the build, they'll soon get the hang of it.

pms1969
  • 3,354
  • 1
  • 25
  • 34
0

The problem with SQL injection is, that a user input is used as part of the SQL statement. By using prepared statements you can force the user input to be handled as the content of a parameter (and not as a part of the SQL command). Query parameters help to avoid this risk by separating literal values from the SQL syntax.

Most client APIs (including .NET) support parameterization of queries. This allows embedding the user input as parameters. The parameters are placeholders for user entered value which is replaced at execution time. That way the user cannot inject SQL code as the whole user entry is treated as value for the parameter, not as string appended to the query.

parameterization is the best solution for SQL injection attacks.

James
  • 51
  • 3