Formatted SQL queries can be difficult to maintain, debug and can increase the risk of SQL injection when concatenating untrusted values into the
query. However, this rule doesn’t detect SQL injections (unlike rule S3649), the goal is only to highlight complex/formatted
queries.
Ask Yourself Whether
- Some parts of the query come from untrusted values (like user inputs).
- The query is repeated/duplicated in other parts of the code.
- The application must support different types of relational databases.
There is a risk if you answered yes to any of those questions.
Recommended Secure Coding Practices
Sensitive Code Example
public void Foo(DbContext context, string query, string param)
{
string sensitiveQuery = string.Concat(query, param);
context.Database.ExecuteSqlCommand(sensitiveQuery); // Sensitive
context.Query<User>().FromSql(sensitiveQuery); // Sensitive
context.Database.ExecuteSqlCommand($"SELECT * FROM mytable WHERE mycol={value}", param); // Sensitive, the FormattableString is evaluated and converted to RawSqlString
string query = $"SELECT * FROM mytable WHERE mycol={param}";
context.Database.ExecuteSqlCommand(query); // Sensitive, the FormattableString has already been evaluated, it won't be converted to a parametrized query.
}
public void Bar(SqlConnection connection, string param)
{
SqlCommand command;
string sensitiveQuery = string.Format("INSERT INTO Users (name) VALUES (\"{0}\")", param);
command = new SqlCommand(sensitiveQuery); // Sensitive
command.CommandText = sensitiveQuery; // Sensitive
SqlDataAdapter adapter;
adapter = new SqlDataAdapter(sensitiveQuery, connection); // Sensitive
}
Compliant Solution
public void Foo(DbContext context, string query, string param)
{
context.Database.ExecuteSqlCommand("SELECT * FROM mytable WHERE mycol=@p0", param); // Compliant, it's a parametrized safe query
}
See
