SQL query built from user-controlled sources¶
ID: cs/sql-injectionKind: path-problemSecurity severity: 8.8Severity: errorPrecision: highTags: - security - external/cwe/cwe-089Query suites: - csharp-code-scanning.qls - csharp-security-extended.qls - csharp-security-and-quality.qls
Click to see the query in the CodeQL repository
If a SQL query is built using string concatenation, and the components of the concatenation include user input, a user is likely to be able to run malicious database queries.
Recommendation¶
Usually, it is better to use a prepared statement than to build a complete query with string concatenation. A prepared statement can include a parameter, written as either a question mark (?) or with an explicit name (@parameter), for each part of the SQL query that is expected to be filled in by a different value each time it is run. When the query is later executed, a value must be supplied for each parameter in the query.
It is good practice to use prepared statements for supplying parameters to a query, whether or not any of the parameters are directly traceable to user input. Doing so avoids any need to worry about quoting and escaping.
Example¶
In the following example, the code runs a simple SQL query in three different ways.
The first way involves building a query,query1, by concatenating a user-supplied text box value with some string literals. The text box value can include special characters, so this code allows for SQL injection attacks.
The second way uses a stored procedure,ItemsStoredProcedure, with a single parameter (@category). The parameter is then given a value by callingParameters.Add. This version is immune to injection attacks, because any special characters are not given any special treatment.
The third way builds a query,query2, with a single string literal that includes a parameter (@category). The parameter is then given a value by callingParameters.Add. This version is immune to injection attacks, because any special characters are not given any special treatment.
usingSystem.Data;usingSystem.Data.SqlClient;usingSystem.Web.UI.WebControls;classSqlInjection{TextBoxcategoryTextBox;stringconnectionString;publicDataSetGetDataSetByCategory(){// BAD: the category might have SQL special characters in itusing(varconnection=newSqlConnection(connectionString)){varquery1="SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='"+categoryTextBox.Text+"' ORDER BY PRICE";varadapter=newSqlDataAdapter(query1,connection);varresult=newDataSet();adapter.Fill(result);returnresult;}// GOOD: use parameters with stored proceduresusing(varconnection=newSqlConnection(connectionString)){varadapter=newSqlDataAdapter("ItemsStoredProcedure",connection);adapter.SelectCommand.CommandType=CommandType.StoredProcedure;varparameter=newSqlParameter("category",categoryTextBox.Text);adapter.SelectCommand.Parameters.Add(parameter);varresult=newDataSet();adapter.Fill(result);returnresult;}// GOOD: use parameters with dynamic SQLusing(varconnection=newSqlConnection(connectionString)){varquery2="SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY="+"@category ORDER BY PRICE";varadapter=newSqlDataAdapter(query2,connection);varparameter=newSqlParameter("category",categoryTextBox.Text);adapter.SelectCommand.Parameters.Add(parameter);varresult=newDataSet();adapter.Fill(result);returnresult;}}}
References¶
Common Weakness Enumeration:CWE-89.