views:

153

answers:

4

The typical controls against SQL injection flaws are to use bind variables (cfqueryparam tag), validation of string data and to turn to stored procedures for the actual SQL layer. This is all fine and I agree, however what if the site is a legacy one and it features a lot of dynamic queries. Then, rewriting all the queries is a herculean task and it requires an extensive period of regression and performance testing. I was thinking of using a dynamic SQL filter and calling it prior to calling cfquery for the actual execution.

I found one filter in CFLib.org (http://www.cflib.org/udf/sqlSafe):

<cfscript>
/**
* Cleans string of potential sql injection.
*
* @param string      String to modify. (Required)
* @return Returns a string.
* @author Bryan Murphy ([email protected])
* @version 1, May 26, 2005
*/
function metaguardSQLSafe(string) {
var sqlList = "-- ,'";
var replacementList = "#chr(38)##chr(35)##chr(52)##chr(53)##chr(59)##chr(38)##chr(35)##chr(52)##chr(53)##chr(59)# , #chr(38)##chr(35)##chr(51)##chr(57)##chr(59)#";

return trim(replaceList( string , sqlList , replacementList ));
}
</cfscript>

This seems to be quite a simple filter and I would like to know if there are ways to improve it or to come up with a better solution?

+9  A: 

what if the site is a legacy one and it features a lot of dynamic queries. Then, rewriting all the queries is a herculean task and it requires an extensive period of regression and performance testing.

Yep, but that's the case if you perform any significant changes, including using a function like the one you are proposing.

So I'd still recommend getting some tests setup, refactoring to use a sensible framework, and then fixing the queries to use cfqueryparam.

That specific function is a bunch of nonsense, which does not do what it claims to do, and has the potential to break stuff (by incorrectly exceeding max lengths).

All it does is turns -- into &#45;&#45; and ' into &#39; - this is not SQL injection protection!

So yeah, if you still do want to go down that route, find a different function, but I'd recommend proper refactoring.

Peter Boughton
You are absolutely correct. My opinion is that refactoring hundreds of queries to make use of cfqueryparam involves a longer testing cycle than re-wiring the existing functions to make use of a central function that sanitizes the input before it gets executed. Well, you may say that at the end of the day this still involves heavy testing and this is true, but at least it is contained centrally. Do you happen to know of a good open-source function (ideally in CF) that does this job?
+3  A: 

Obviously you have a lot of work ahead of you. But as you roll up your sleeves, one small thing you might do to mitigate some of the potential damage from injection attacks is to create several datasources, and run all your select-only queries through a datasource restricted to only select statements. And for all of the datasources, make sure things like grant, revoke, create, alter, and drop are disabled.

Ken Redler
A: 

You might try Portcullis. It is an open source CFC that you can use to scan the URL, FORM and COOKIE scopes for SQL Injection and XSS attacks. It won't be guaranteed protection but would at least provide some protection today with little effort while you work on a rewrite of the queries. The nice thing is it can be included in the Application.cfm/cfc to scan the scopes on every CF page request at the cost of about 4 lines of code.

shooksm
A: 

Put this coding into your application.cfm file.

<cfif FindNoCase(“DECLARE”,cgi.query_string) and FindNoCase(“CAST”,cgi.query_string) and FindNoCase(“EXEC”,cgi.query_string)> <cfabort showerror="Oops..!! It's SQL injection." > </cfif>

http://ppshein.wordpress.com/2008/08/23/sql-injection-attacks-by-store-procedure/

http://ppshein.wordpress.com/2008/08/28/block-ip-in-coldfusion/

ppshein