Review commands vulnerable to SQL injection

Rule ND3107 Security

Matches methods creating a DB command (like SqlCommand) that call an Execute method without using command parameters: prone to SQL Injection https://en.wikipedia.org/wiki/SQL_injection since malicious SQL might be injected in string parameter values.

False positives are possible; review matches and suppress issues when needed. To limit them, the rule also checks command parameters access from sub method calls (a solid indication of non-vulnerability).

Flagged Code
public int CountUsers(SqlConnection connection, string name) {
   var sql = "SELECT COUNT(*) FROM Users WHERE Name = '" + name + "'";
   using var command = new SqlCommand(sql, connection);  // flagged: no command parameter
   return (int)command.ExecuteScalar();
}
Fixed Code
public int CountUsers(SqlConnection connection, string name) {
   const string sql = "SELECT COUNT(*) FROM Users WHERE Name = @name";
   using var command = new SqlCommand(sql, connection);
   command.Parameters.AddWithValue("@name", name);  // parameterized: not matched
   return (int)command.ExecuteScalar();
}

How to fix

If the method indeed injects parameter values in the SQL string, use command.Parameters.Add(...) instead. More on explicit parameters: https://stackoverflow.com/questions/4892166/how-does-sqlparameter-prevent-sql-injection

CQLinq source

This rule is written in CQLinq, C# LINQ over the NDepend code model. It ships with every new NDepend project: you can read it, adapt it, or use it as a starting point for your own custom rules.

// <Name>Review commands vulnerable to SQL injection</Name>
// <Id>ND3107:ReviewCommandsVulnerableToSQLInjection</Id>

warnif count > 0

let commands = ThirdParty.Types.WithFullNameIn(
  "System.Data.Common.DbCommand",
  "Microsoft.Data.SqlClient.SqlCommand",
  "System.Data.SqlClient.SqlCommand",
  "System.Data.OleDb.OleDbCommand",
  "System.Data.Odbc.OdbcCommand",
  "System.Data.OracleClient.OracleCommand")
where commands.Any()
let commandCtors = commands.ChildMethods().Where(m => m.IsConstructor).ToHashSetEx()
let commandExecutes = commands.ChildMethods().Where(m => m.SimpleName.Contains("Execute")).ToHashSetEx()
let commandParameters = commands.ChildMethods().Where(m => m.IsPropertyGetter && m.SimpleNameEquals("get_Parameters")).ToHashSetEx()


from m in Application.Methods.UsingAny(commandCtors)
                             .UsingAny(commandExecutes)
where !m.MethodsCalled.Intersect(commandParameters).Any() &&
      // Check also that non of the method call rely on command parameters to get less false positives: 
      !m.MethodsCalled.SelectMany(mc => mc.MethodsCalled).Intersect(commandParameters).Any()

select new { m, 
   createA = m.MethodsCalled.Intersect(commandCtors).First().ParentType,
   calls = m.MethodsCalled.Intersect(commandExecutes).First(),
   Debt = 15.ToMinutes().ToDebt(),
   Severity = Severity.High
}

//<Expl>
// The *{0}* creates an instance of the *{1}* and calls the *{2}*.
// This is susceptible to *SQL Injection* as malicious SQL code 
// can be injected into string parameter values. 
// Instead, use *command.Parameters.Add(...)*.
//</Expl>

// <Desc>
// Matches methods creating a DB command (like *SqlCommand*) that call an
// *Execute* method without using command parameters: prone to
// **SQL Injection** https://en.wikipedia.org/wiki/SQL_injection since
// malicious SQL might be injected in string parameter values.
//
// False positives are possible; review matches and suppress issues when
// needed. To limit them, the rule also checks command parameters access
// from sub method calls (a solid indication of non-vulnerability).
//
// **Flagged Code**
// ```csharp
// public int CountUsers(SqlConnection connection, string name) {
//    var sql = "SELECT COUNT(*) FROM Users WHERE Name = '" + name + "'";
//    using var command = new SqlCommand(sql, connection);  // flagged: no command parameter
//    return (int)command.ExecuteScalar();
// }
// ```
//
// **Fixed Code**
// ```csharp
// public int CountUsers(SqlConnection connection, string name) {
//    const string sql = "SELECT COUNT(*) FROM Users WHERE Name = @name";
//    using var command = new SqlCommand(sql, connection);
//    command.Parameters.AddWithValue("@name", name);  // parameterized: not matched
//    return (int)command.ExecuteScalar();
// }
// ```
// </Desc>

// <HowToFix>
// If the method indeed injects parameter values in the SQL string, use
// **command.Parameters.Add(...)** instead. More on explicit parameters:
// https://stackoverflow.com/questions/4892166/how-does-sqlparameter-prevent-sql-injection
// </HowToFix>

Open in the interactive Rules Explorer · All default rules

Security: all 39 items