We’re all aware of the concerns of SQL Injection these days. Lots of articles written about over the years. ColdFusion back in 6.1 I believe even came out with a nice tag called cfqueryparam which indeed helps.
My issue is with the ‘Order By’ clause. It seems that since you don’t have a nice neat little tag such as cfqueryparam to use here, you are on your own to protect yourself. I’m willing to bet there are several applications out there where the developer just plugged in cfqueryparam into the where clause when needed and yet still exposed the order by statement.
Example:
Select name, address, dateadded
From tablename
Where ID = <cfqueryparam cfsqltype=”CF_SQL_INTEGER” value=”#url.sort#” />
ORDER BY #URL.SORT#
What’s to prevent the hacker from typing into the url “index.cfm?ID=25&sort=fieldname;delete from tablename”
I brought this up on the cf-talk message board and you can read it here.
The best solution in the most basic form as far as I can tell is to abstract the field names from the client and use cfif or case/switch like the following:
SELECT name, address, dateadded
From tablename
Where ID = <cfqueryparam cfsqltype=”CF_SQL_INTEGER” value=”#arguments.InventoryID#” />
ORDER BY
<cfswitch expression=”#url.sort#”>
<cfcase value=”a”>
name
</cfcase>
<cfcase value=”b”>
address
</cfcase>
<cfdefaultcase>
dateadded
</cfdefaultcase>
</cfswitch>
This insures you are testing for exactly the field name you expect and that you send the correct information to the database. Be sure to use cfdefaultcase in the event that none is passed.
I’d love you hear your thoughts and experiences on this. I’m especially interested to see if you have a more elegant or concise way of doing this. Especially if you have 10 plus column names and want the option to reverse search. The switch will get huge and ugly with my example.
Vince Collins
September 17, 2007 at 6:06 pm
That is how I do it as well. I think it is the best solution (that I have found) for two reasons:
1. It allows you to protect against SQL injection attacks which is sweet.
2. It allows you to do a secondary sort. For example, in the above, you can sort by dateadded as well as name. Well, maybe when you sort by dateadded, you then want to do a second sort on name ASC (or something to that effect).
As far as getting huge an ugly, you can always just param the column in some way. Like do something like :
CFIF NOT ListFind( “name,address,dateadded”, URL.sort )
CFSET URL.sort = name
/CFIF
But, then again, doing it this way, I still think you lose out on #2 above, so it depends on what you need to do in the long run.
September 17, 2007 at 7:13 pm
I normally don’t use CFQUERYPARAM at all… if a non-integer is passed and CFQUERYPARAM caught it and threw an error, the page would still crash and I’d still get a notification in my inbox.
My goal is to prevent the error before it happens — so the page doesn’t crash and I don’t get a notification. So I setup a UDF which validates all URL parameters. It strips out anything that is not of the intended type, and anything following a “;”, thus returning only a valid value. Then the query, and the rest of the page, can continue on without additional worries.
Part of my rationale is that I often get errors where the mangled URL parameter is a copy & paste error by a website user - not an injection. So I believe it’s important to handle the error, not just throw it.
September 18, 2007 at 8:32 am
A simple but effective solution is to use:
order by #listFirst(url.orderBy,’;')
This allows for multiple column sorting but dumps injection attempts.
A better solution that requires a bit more code is to check for the ‘;’ character before running the query and if it exists don’t run the query at all.
September 18, 2007 at 9:51 am
Ray, you are of course correct - it is better to handle errors properly rather than just throw it - but that doesn’t mean you shouldn’t use cfqueryparam; there are still two good reasons to use it:
1: What if you miss something in your data cleansing routine? It’s always sensible to have a backup plan.
2: CFQP allows bind variables to be used, which means that databases can do things more optimally, improving performance.
September 18, 2007 at 10:23 am
I like the short-hand solution for simple queries but as Ben mentions, there are times when you want to sort by more than one field.
Say I want to sort by
LastName (ASC,DESC)
FirstName (ASC,DESC)
Date (ASC,DESC)
LastName, FirstName, (ASC,DESC)
You pretty much have to write it all out to allow for this I think.
Select * from Users
Order by
cfswitch expression=”#url.sort#”
cfcase value=”Last_name”
LastName
/cfcase
cfcase value=”Last_Name Desc”
LastName Desc
/cfcase
cfcase value=”First_name”
FirstName
/cfcase
cfcase value=”First_Name Desc”
FirstName Desc
/cfcase
cfcase value=”Date”
Date
/cfcase
cfcase value=”Date Desc”
Date Desc
/cfcase
cfcase value=”Last_Name, First_Name, Date Desc”
LastName, FirstName, Date Desc
/cfcase
cfcase value=”Last_Name Desc, First_Name Desc, Date Desc”
LastName Desc, FirstName Desc, Date Desc
/cfcase
/cfswitch
This abstracts the names of the fields in the DB (well, kind of) as well as gives you the ability to sort in any which way you want. I wish there was a cleaner way but at least it works. And, with CFCs, you build it once and you are done. Well, once for each database query…
September 18, 2007 at 10:25 am
Ack! wordpress stripped out my tags…
September 18, 2007 at 10:30 am
OK, comments from others appear to strip out the html tags. I “fixed” it by just not using the gt/lt brackets.
September 18, 2007 at 10:33 am
I agree with Peter. cfqueryparam is very useful and should be used as only PART of your overall Injection strategy such as Ray’s other ‘cleansing’ routines and error handling.