Filed under: Development, Security, Testing
It is not news that SQL Injection is possible within a stored procedure. There have been plenty of articles discussing this issues. However, there is a unique way that some developers execute their stored procedures that make them vulnerable to SQL Injection, even when the stored procedure itself is actually safe.
Look at the example below. The code is using a stored procedure, but it is calling the stored procedure using a dynamic statement.
conn.Open(); var cmdText = "exec spGetData '" + txtSearch.Text + "'"; SqlDataAdapter adapter = new SqlDataAdapter(cmdText, conn); DataSet ds = new DataSet(); adapter.Fill(ds); conn.Close(); grdResults.DataSource = ds.Tables; grdResults.DataBind();
It doesn’t really matter what is in the stored procedure for this particular example. This is because the stored procedure is not where the injection is going to occur. Instead, the injection occurs when the EXEC statement is concatenated together. The email parameter is being dynamically added in, which we know is bad.
This can be quickly tested by just inserting a single quote (‘) into the search field and viewing the error message returned. It would look something like this:
System.Data.SqlClient.SqlException (0x80131904): Unclosed quotation mark after the character string ”’. at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction) at System.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction) at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose) at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady) at System.Data.SqlClient.SqlDataReader.TryConsumeMetaData() at System.Data.SqlClient.SqlDataReader.get_MetaData() at
With a little more probing, it is possible to get more information leading us to understand how this SQL is constructed. For example, by placing ‘,’ into the search field, we see a different error message:
System.Data.SqlClient.SqlException (0x80131904): Procedure or function spGetData has too many arguments specified. at System.Data.SqlClient.SqlConnection.
The mention of the stored procedure having too many arguments helps identify this technique for calling stored procedures.
With SQL we have the ability to execute more than one query in a given transaction. In this case, we just need to break out of the current exec statement and add our own statement. Remember, this doesn’t effect the execution of the spGetData stored procedure. We are looking at the ability to add new statements to the request.
Lets assume we search for this:
firstname.lastname@example.org’;SELECT * FROM tblUsers–
this would change our cmdText to look like:
exec spGetData’email@example.com’;SELECT * FROM tblUsers–‘
The above query will execute the spGetData stored procedure and then execute the following SELECT statement, ultimately returning 2 result sets. In many cases, this is not that useful for an attacker because the second table would not be returned to the user. However, this doesn’t mean that this makes an attack impossible. Instead, this turns our attacks more towards what we can Do, not what can we receive.
At this point, we are able to execute any commands against the SQL Server that the user has permission too. This could mean executing other stored procedures, dropping or modifying tables, adding records to a table, or even more advanced attacks such as manipulating the underlying operating system. An example might be to do something like this:
firstname.lastname@example.org’;DROP TABLE tblUsers–
If the user has permissions, the server would drop tblUsers, causing a lot of problems.
When calling stored procedures, it should be done using command parameters, rather than dynamically. The following is an example of using proper parameters:
conn.Open(); SqlCommand cmd = new SqlCommand(); cmd.CommandText = "spGetData"; cmd.CommandType = CommandType.StoredProcedure; cmd.Connection = conn; cmd.Parameters.AddWithValue("@someData", txtSearch.Text); SqlDataAdapter adapter = new SqlDataAdapter(cmd); DataSet ds = new DataSet(); adapter.Fill(ds); conn.Close(); grdResults.DataSource = ds.Tables; grdResults.DataBind();
The code above adds parameters to the command object, removing the ability to inject into the dynamic code.
It is easy to think that because it is a stored procedure, and the stored procedure may be safe, that we are secure. Unfortunately, simple mistakes like this can lead to a vulnerability. Make sure that you are properly making database calls using parameterized queries. Don’t use dynamic SQL, even if it is to call a stored procedure.
Filed under: Security
One of the most common suggestions for remediating SQL Injection vulnerabilities is to use stored procedures. Using stored procedures can help decrease the risk of SQL Injection, but if implemented incorrectly, it can create a false sense of security. For example, look at the created stored procedure below:
CREATE PROCEDURE dbo.usp_IsValidUser ( @UserName varchar(50), @Password varchar(50) ) AS SET NOCOUNT ON SELECT MemberId FROM Member WHERE UserName = @UserName AND Password = @Password RETURN
This looks like a pretty secure stored procedure. It does not use any dynamic SQL, or the dreaded EXEC command. It is still recommended that you validate your inputs, but the parameters should not execute as SQL code, but instead just as strings.
Lets take a look at the wrong way to call a stored procedure below:
_command.CommandType = CommandType.Text; _command.CommandText = "EXEC usp_IsValidUser '" + UserName + "', '" + Password + "'";
In this example, although calling a stored procedure, is still vulnerable to SQL Injection (if proper input validation is not performed). The stored procedure may be safe, but the developer has just created an inline query to call the stored procedure. A malicious user could append SQL queries to the end of this query to run their own queries.
For example, if the user passes in a Password value that contains the single quote (‘), then they can dump out of the password parameter and append on their own queries. It is beyond this post to show detailed examples of exploiting this type of statement.
It is important for developers to keep this scenario in mind when developing and doing peer code reviews. Any time the code is appending data to send to the data store you should investigate further. Always validate the input received from un-trusted sources. Use Parameterized queries instead of string building to create database queries.
Filed under: Security
Yesterday, Microsoft released two new Quick Security References (QSR’s) to help application development teams understand Security issues. These new guides are the first part of a continuing series to help multiple roles within the team understand common vulnerabilities. Not only do they provide great detail on the security issues, but they also help teams move toward SDL adoption.
The first two QSR’s focus on Cross Site Scripting and SQL Injection. I think it is good that they started with these two vulnerabilities because they are the two most common types of attacks. These two vulnerabilities take turns in the first and second position on the OWASP Top 10. I encourage anyone and everyone involved with applications, from the business personnel to the technical teams, to read over these guides. They are about 20 pages in length, but provide a really good description of the attacks.
The QSR’s can be downloaded from Microsoft here: http://www.microsoft.com/downloads/details.aspx?FamilyID=79042476-951f-48d0-8ebb-89f26cf8979d&displayLang=en