Rob Kraft's Software Development Blog

Software Development Insights

C# .Net SQL Injection Detection – Especially for Legacy .Net Code

Posted by robkraft on March 4, 2020

When you have an existing .Net code base full of SQL statements, and you want to reduce the chance that there are SQL injection risks in the code, you may decide to perform a review of every SQL statement in order to confirm that they are all coded correctly; or you may hire another company to do this for you. But one problem with this approach is that the code is only “SQL Injection Free” from the moment the review is completed until people start modifying and adding to the code again.

What you should strive for is a way to make sure every past and future SQL statement gets tested for SQL Injection risk before it runs.  That is what this sample code provides you.  If you follow the patterns described here, I believe you can significantly reduce the risk that your code has bugs leading to SQL Injection and it will stay that way going forward.

Using the Decorator Pattern to Provide a Place To Add SQL Injection Detection

The primary technique I recommend in this article for adding SQL Injection detection into your application is to stop using the .ExecuteReader and .ExecuteNonQuery methods.  Instead, use the Decorator pattern to create your own method to be called in place of those two, and that method will include code to do some SQL Injection detection.

Replace:

SqlDataReader reader = command.ExecuteReader();

With 

SqlDataReader reader = command.ExecuteSafeReader(); //provided in sample code

The sample code provided behaves like the Proxy pattern in that it will make the actual call to the database after finding no SQL Injection risk.  The benefit of this approach is that you can then regularly scan your entire code base for the use of .ExecuteReader and .ExecuteNonQuery knowing that there should be no cases of those methods, other than the exception cases you expect.  Thus you can be sure that the majority of your code is running through your SQL Injection detector.

Another benefit of using the Decorator pattern to implement SQL Injection Detection is that you can also easily add other features such as:

  • Logging every SQL that is executed
  • Logging and blocking every SQL that is a SQL Injection risk
  • Altering every SQL on the fly.  One scenario where this could be helpful is that if you renamed a table in the database but had a lot of SQL that needed to change.  You could possibly add a find/replace to every SQL on the fly to change the table name, allowing you more time to find and correct all stored SQL fragments with the old table name.
	public static SqlDataReader ExecuteSafeReader(this SqlCommand sqlcommand)
	{
		if (!sqlcommand.CommandType.Equals(CommandType.StoredProcedure))
		{
			var sql = sqlcommand.CommandText;
			//Options: You could Add logging of the SQL here to track every query ran
			//Options: You could edit SQL - for example if you had renamed a table in the database
			if (!ValidateSQL(sql, SelectRegex))
				return null;
		}

		return sqlcommand.ExecuteReader();
	}

The SQL Injection Detection Code

Warning!  This does not detect all forms of SQL Injection, but it will detect most of them.  Here is what causes the class to throw an exception:

  • Finding a single apostrophe (single quote) that does not have a matching single apostrophe (single quote)
  • Finding double quotes that do not have a matching double quote.  This is only needed if the SQL Server has SET QUOTED_IDENTIFIER OFF.  However, you may also want to use this if your database is MySQL or some other DBMS.
  • Finding a comment within the SQL
  • Finding an ASCII value great than 127
  • Finding a semicolon
  • After extracting the strings and comments, finding any of a specific configurable list of keywords in a SELECT statement such as DELETE, SYSOBJECTS, TRUNCATE, DROP, XP_CMDSHELL

The code is written to be easy to change if you don’t want to enforce any of the rules above, or if you need to add similar rules because you have a special scenario or a DBMS besides SQL Server.

The code uses the regex [^\u0000-\u007F] to reject the SQL if it contains any non-ASCII characters.  This works for the applications I have written, but may need alteration for non American English language support.

The code also uses regexes to check SQL statements for undesirable keywords.  One regex is for SELECT statements and therefore blocks them if they contain INSERT, UPDATE, or DELETE.  Other keywords that may indicate a SQL Injection attempt are also rejected and that list includes waitfor, xp_cmdshell, and information_schema.  Note that I also include UNION in the list; so if you use the UNION keyword you will need to remove that from the list.  UNION is frequently used by hackers attempting to perform SQL Injection.

private static void LoadFromConfig()
{

	_asciiPattern = "[^\u0000-\u007F]";
	_selectpattern = @"\b(union|information_schema|insert|update|delete|truncate|drop|reconfigure|sysobjects|waitfor|xp_cmdshell)\b|(;)";
	_modifypattern = @"\b(union|information_schema|truncate|drop|reconfigure|sysobjects|waitfor|xp_cmdshell)\b|(;)";
	_rejectIfCommentFound = true;
	_commentTagSets = new string[2, 2] { { "--", "" }, { "/*", "*/" } };
}

SQL Server supports two techniques to comment out SQL code in a SQL Statement, two dashes, and enclosing the comment in /* */.  Since it is unlikely that developers write SQL to include comments, my default choice is to reject any SQL containing those values.

Exactly How Is The SQL Injection Detected?

There are basically three steps in the SQL Injection detection process.

First, the code checks for any ASCII values above 127 and rejects the SQL if one is found.

Second, the code removes all the code withing strings and comments.  So an SQL that starts out looking like this:

select * from table where x = ‘ss”d’ and r = ‘asdf’ /* test */ DROP TABLE NAME1 order by 5

becomes this:

select * from table where x = and r = t DROP TABLE NAME1 order by 5

Third, the code looks for keywords, like “DROP” and “XP_CMDSHELL”, in the revised SQL that are on the naughty list.  If any of those keywords are found, the SQL is rejected.

Formatting Methods included in the SQLExtensions Class

The SQLExtensions class provides additional methods to help your coders reduce the risk of SQL Injection.  These methods help coders format variables in SQL when doing so with a parameter is not an option.  The most useful of these methods is FormatStringForSQL and it could be used as shown here to enclose a string in SQL quotes as well as replace any single quotes contained within the value with two single quotes.


string sql = "select * from customers where firstname like " + nameValue.FormatStringForSQL();

Another advantage of using a method like this is that it makes it easy for you to change how you handle the formatting of strings everywhere within your code if you discover that you need to make a change.  For example, perhaps you decide to move your application from SQL Server to MySQL and therefore that you also need to replace double quotes in addition to single quotes.  You could make the change within this method instead of reviewing your entire code base to make the change one by one for each SQL.

Custom .Net Exception Class

I also provided a custom Exception primarily to show how easy it is to implement custom exceptions and because I think it is useful for this extension class.  This provides you more flexibility for handling exceptions.  You can catch and handle the exceptions raised specifically due to SQL Injection risk different than exceptions thrown by the underlying ADO.NET code returned from the database.


[Serializable]
public class SQLFormattingException : Exception
{
	public SQLFormattingException() {}

	public SQLFormattingException(string message): base(message) {}
}

The Rules For Detecting SQL Injection are Configurable

I made enabling/disabling configuration of the SQL Injection detections easy to change so that you could import those rules at runtime if desired so that different applications could have different rules.  Perhaps one of your applications needs to allow semicolons in SQL but the others don’t.  It is a good practice to implement the most stringent rules you can everywhere you can.  Don’t implement weak SQL Injection detection rules everywhere because a single place in your code needs weaker rules.  The rules are “Lazy Loaded” when needed, then cached, to support the ability to change them while an application is running by calling the InvalidateCache method provided.

Below is an example of one of the rules.  You can configure your code to reject the SQL if it contains SQL Server comments.


#region RejectComments Flag
private static bool? _rejectIfCommentFound = null;
public static bool RejectIfCommentFound
{
	get
	{
		if (_rejectIfCommentFound == null)
		{
			LoadFromConfig();
		}
		return (bool)_rejectIfCommentFound;
	}
}
#endregion

Steps To Implement and Use This Code

I suggest you take the following steps to implement this class:

  1. Get the SQLExtensions.cs class file into a project in your code base. You will also need the CustomExceptions.cs class file.  The program.cs just contains a sample usage and there is also a UnitTest1.cs class.
  2. Comment out all the lines in ReallyValidateSQL except for the “return true”
  3. Do a find and replace across your entire code base to replace ExecuteReader with ExecuteSafeReader
  4. Compile and test.  Your app should still work exactly the same at this point.
  5. Review the Customizable Validation Properties and decided which ones you want to implement, then uncomment the lines you commented out in ReallyValidateSQL
  6. Decide if you need to and want to replace dynamically constructed SQL in your application with any of the four FormatSQL… extension methods provided.
  7. Provide me feedback

MIT FREE TO USE LICENSE

This code has an MIT license which means you can use this code in commercial products for free!

A link to the source code example is here: https://github.com/RobKraft/SQLInjectionDetection

2 Responses to “C# .Net SQL Injection Detection – Especially for Legacy .Net Code”

  1. RichardD said

    If you’re going to have to go through your code to replace every call to ExecuteReader, ExecuteScalar, and ExecuteNonQuery with a custom method, it would be far better to replace the string concatenation which lead to the SQL Injection vulnerability with a properly parameterized query instead.

    As you say, there’s no way this can detect all forms of SQL Injection. It’s just going to give people a false sense of security instead.

    • robkraft said

      Thanks for your input Richard. I recommend doing both. Changing the code to use parameters and also implementing this technique. This technique provides a good fallback for catching SQL Injections for cases that can’t support parameters, such as dynamic ORDER BY clauses and dynamic columns entered into the SQL. Also, it is very easy to implement this. You can implement this for your entire code base in an hour. Changing all the code to use parameters can take months and also introduce errors.
      But you are correct, people need to understand, as I mention in this article, that this is no guarantee to totally protect your application from SQL Injection.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

 
%d bloggers like this: