Export (0) Print
Expand All
Expand Minimize

CA2100: Review SQL queries for security vulnerabilities

TypeName

ReviewSqlQueriesForSecurityVulnerabilities

CheckId

CA2100

Category

Microsoft.Security

Breaking Change

Non-breaking

A method sets the IDbCommand::CommandText property by using a string that is built from a string argument to the method.

This rule assumes that the string argument contains user input. A SQL command string that is built from user input is vulnerable to SQL injection attacks. In a SQL injection attack, a malicious user supplies input that alters the design of a query in an attempt to damage or gain unauthorized access to the underlying database. Typical techniques include injection of a single quotation mark or apostrophe, which is the SQL literal string delimiter; two dashes, which signifies a SQL comment; and a semicolon, which indicates that a new command follows. If user input must be part of the query, use one of the following, listed in order of effectiveness, to reduce the risk of attack.

  • Use a stored procedure.

  • Use a parameterized command string.

  • Validate the user input for both type and content before you build the command string.

The following .NET Framework types implement the CommandText property or provide constructors that set the property by using a string argument.

Notice that this rule is violated when the ToString method of a type is used explicitly or implicitly to construct the query string. The following is an example.

int x = 10;
string query = "SELECT TOP " + x.ToString() + " FROM Table";

The rule is violated because a malicious user can override the ToString() method.

The rule also is violated when ToString is used implicitly.

int x = 10;
string query = String.Format("SELECT TOP {0} FROM Table", x);

To fix a violation of this rule, use a parameterized query.

It is safe to suppress a warning from this rule if the command text does not contain any user input.

The following example shows a method, UnsafeQuery, that violates the rule and a method, SaferQuery, that satisfies the rule by using a parameterized command string.

#using <System.dll>
#using <System.Data.dll>
#using <System.EnterpriseServices.dll>
#using <System.Transactions.dll>
#using <System.Xml.dll>
using namespace System;
using namespace System::Data;
using namespace System::Data::SqlClient;

namespace SecurityLibrary
{
   public ref class SqlQueries
   {
   public:
      Object^ UnsafeQuery(
         String^ connection, String^ name, String^ password)
      {
         SqlConnection^ someConnection = gcnew SqlConnection(connection);
         SqlCommand^ someCommand = gcnew SqlCommand();
         someCommand->Connection = someConnection;

         someCommand->CommandText = String::Concat(
            "SELECT AccountNumber FROM Users WHERE Username='", 
            name, "' AND Password='", password, "'");

         someConnection->Open();
         Object^ accountNumber = someCommand->ExecuteScalar();
         someConnection->Close();
         return accountNumber;
      }

      Object^ SaferQuery(
         String^ connection, String^ name, String^ password)
      {
         SqlConnection^ someConnection = gcnew SqlConnection(connection);
         SqlCommand^ someCommand = gcnew SqlCommand();
         someCommand->Connection = someConnection;

         someCommand->Parameters->Add(
            "@username", SqlDbType::NChar)->Value = name;
         someCommand->Parameters->Add(
            "@password", SqlDbType::NChar)->Value = password;
         someCommand->CommandText = "SELECT AccountNumber FROM Users "   
            "WHERE Username=@username AND Password=@password";

         someConnection->Open();
         Object^ accountNumber = someCommand->ExecuteScalar();
         someConnection->Close();
         return accountNumber;
      }
   };
}

using namespace SecurityLibrary;

void main()
{
   SqlQueries^ queries = gcnew SqlQueries();
   queries->UnsafeQuery(Environment::GetCommandLineArgs()[1], 
      "' OR 1=1 --", "anything");
   // Resultant query (which is always true):  
   // SELECT AccountNumber FROM Users WHERE Username='' OR 1=1

   queries->SaferQuery(Environment::GetCommandLineArgs()[1], 
      "' OR 1 = 1 --", "anything");
   // Resultant query (notice the additional single quote character): 
   // SELECT AccountNumber FROM Users WHERE Username=''' OR 1=1 --' 
   //                                   AND Password='anything'
}
Show:
© 2014 Microsoft