SQL Injection: Common Mistake

Posted by on September 1, 2010

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.

		
		

		
		

				
		
		
		

			
Tags: ,  

		

					
				
		
Comments

		





			
		
Comments are closed.