Posted by Brad Wood
Jul 22, 2008 13:31:00 UTC
JR asked a good question on my queryparam Scanner post. He noticed that I had stopped short of saying cfqueryparam would ALWAYS stop ALL SQL injection. He said, "Can you give an example of a SQL Injection attack which is not caught by cfqueryparam ?" I'm glad you asked JR.SQL injection can occur any time cautions are not taken to completely separate your SQL code from its parameters and the parameters being passed in are allowed to flow into the SQL statement itself. SQL injection can also occur when building the SQL statement's table and column names dynamically. Here is an example in which every input into the cfquery is parameterized, but SQL injection can still occur:
[code]<cfquery name="qry_hack_me" datasource="foo"> DECLARE @var AS varchar(50) DECLARE @sql_string AS varchar(200) SET @var = <cfqueryparam value="#url.id#"> SET @sql_string = 'SELECT column ' SET @sql_string = @sql_string + 'FROM table t ' SET @sql_string = @sql_string + 'WHERE column2 = ' + chr(39) + @var + chr(39) EXEC @sql_string </cfquery> [/code]AS you can see, the only way the parameter gets into the query is through the cfqueryparam. The SQL statement, however, takes that parameter and unscrupulously mixes it in with SQL code in a manner that would allow injection to occur. The EXEC and EXECUTE statements not differentiate between SQL and parameters since it is just all one big string as far as it is concerned. If the @var variable contains a single tick, it will "break out" into the rest of the statement. The example above is obviously simplified to prove the point. There would be no reason to do exactly what I did, but people often find reasons to do stuff like that. There is a work around though. MSSQL also gives us the system proc sp_executesql. Here's what it would look like:
[code]<cfquery name="qry_unhackable" datasource="foo"> DECLARE @var AS varchar(50) DECLARE @sql_string AS nvarchar(200) DECLARE @param_def AS nvarchar(200) SET @var = <cfqueryparam value="#url.id#"> SET @sql_string = N'SELECT column ' SET @sql_string = @sql_string + N'FROM table t ' SET @sql_string = @sql_string + N'WHERE column2 = @use_this_id' SET @param_def = N'@use_this_id varchar(50)' EXECUTE sp_executesql @sql_String, @parm_def, @use_this_id = @Var </cfquery> [/code]sp_executesql allows you to concoct your dynamic SQL statement, but still keep your parameters separate from the statement itself until execution. If you are on MSSQL and you MUST dynamically build a statement on the Database side, please consider sp_executesql. The other common way cfqueryparam won't help you is if you have a stored procedure with dynamic SQL in which the table or column names themselves are dynamic. Additionally the order by clause is commonly made dynamic. While code like that can be VERY flexible, I would suggest refactoring your application to not require it. If your app unavoidably requires a SQL statement build with random tables, NEVER accept input directly in. Check systables or INFORMATION_SCHEMA with a parameterized query first to make sure that is a valid table or column name. Alternatively, instead of passing variables directly into the statement, use the variable in an if statement of predefined choices so in the worst case scenario your default case will be used, but the incoming variable itself never makes it into the statement. Consider the following stored procedure which may very well have been called with the uber-safe cfstoredproc and cfprocparam tags:
[code]CREATE PROCEDURE sp_test @order_by varchar(4) = 'ASC' -- populated from form input as ASC or DESC AS BEGIN DECLARE @SQL_STRING AS varchar(200) SET @sql_string = 'SELECT column FROM table ORDER by column ' + @order_by EXEC @sql_string END [/code]This procedure is indeed vulnerable. A parameterized query won't solve it this time because the ASC or DESC part of the ORDER BY clause isn't a parameter. (Note, that declaring @order_by as a varchar(4) will definitely mitigate the impact though) In this case, it would be more code, but safer to do the following:
[code]CREATE PROCEDURE sp_test @ORDER_BY varchar(4) = 'ASC' -- populated from form input as ASC or DESC AS BEGIN DECLARE @SQL_STRING AS varchar(200) SET @sql_string = 'SELECT column FROM table ORDER by column ' IF @order_by = 'ASC' BEGIN SET @sql_string = @sql_string + 'ASC' END ELSE BEGIN SET @sql_string = @sql_string + 'DESC' END EXEC @sql_string END [/code]In this way, the parameters to the proc are used to control the SQL statement, but they aren't allowed to enter the statement. There can be endless combinations to this-- many of them exist inside stored procedures-- all of them equally as vulnerable. You can feel safe about using cfqueryparam and cfstoredproc/param, but please don't do it just because I tell you to or someone else tells you to. Do it because you understand WHY SQL injection works and HOW cfqueryparam can (and can't) stop it.