|
|
Home /
Groups /
ColdFusion Talk (CF-Talk)
Sorting Query - SQL Injection
Am I not able to use cfqueryparam in the order by column.Vince Collins 09/17/07 12:35 P You can't use params to control the structure of the statement, onlyBarney Boisvert 09/17/07 12:39 P Thanks Barney,Vince Collins 09/17/07 12:50 P Yeah, I didn't mean to imply that you shouldn't be concerned aboutBarney Boisvert 09/17/07 01:02 P This is all to common even among experienced programmers. I recentlyWil Genovese 09/17/07 01:25 P Thanks Wil,Vince Collins 09/17/07 02:26 P Vince Collins wrote:Jochem van Dieten 09/17/07 02:37 P Jochem, good suggestion, then I can test for a numeric value.Vince Collins 09/17/07 02:54 P I dont like using the order by 2 stuff simply because if the query hasGreg Morphis 09/17/07 03:29 P I agree with you on this.Vince Collins 09/17/07 03:38 P Actually, I was thinking I could just test for the existence of aVince Collins 09/17/07 04:44 P You could also pass the order by via URL, and that would cut theGreg Morphis 09/17/07 02:55 P Yes, I did this in each case statement.Wil Genovese 09/17/07 02:50 P > That suggestion works. However, in an example of a query with 10Brian Kotek 09/17/07 05:46 P Not sure if your DB supports this but with Oracle I can select theGreg Morphis 09/17/07 05:53 P Most RDBMSs support something like this. But you'd want to cache the resultsBrian Kotek 09/17/07 06:31 P > Then in the cfswitch statement I have something like thisCasey Dougall 09/17/07 03:42 P > I think this is how I would go about the new order by stuff .Dave Watts 09/17/07 03:56 P > > <cfset NewSort = component.method(qOrderField=Val(name)) />Casey Dougall 09/17/07 06:46 P Thanks everyone for your responses,Vince Collins 09/18/07 08:36 A > Actually, I was thinking I could just test for the existenceDave Watts 09/17/07 04:57 P I figured :)Vince Collins 09/17/07 05:17 P Am I not able to use cfqueryparam in the order by column. Select * from tablename order by <cfqueryparam cfsqltype="CF_SQL_VARCHAR" value="#SORTCOLUMN#"> The error I'm getting on a windows 2003 server running CF7 is: /[Macromedia][SQLServer JDBC Driver][SQLServer]The SELECT item identified by the ORDER BY number 1 contains a variable as part of the expression identifying a column position. Variables are only allowed when ordering by an expression referencing a column name./ If I just have to do the following... Select * from tablename order by #sortcolumn# ...then my SQL call is not secure from SQL injection. Does this mean I need to write my own tests for the "sortcolumn" variable being passed or create a case statement which then inserts the correct column name? OR...the scenario that is MUCH more likely, am I missing something? You can't use params to control the structure of the statement, only the values passed into the statement. If you're concerned about injection, you can attempt to clean the value before inlining it, or use a CF conditional to emit static stuff: <cfif sortcolumn EQ "name">name<cfelse>age</cfif> cheers, barneyb ----- Excess quoted text cut - see Original Post for more ----- Thanks Barney, I have to say though that I am willing to bet there are programmers that think their SQL is safe from injection just by using the cfqueryparam and forgetting about the order by clause if they allow that to be passed... select * from tablename order by #columnname# index.cfm?columnname=blah;delete from tablename OUCH Barney Boisvert wrote: ----- Excess quoted text cut - see Original Post for more ----- Yeah, I didn't mean to imply that you shouldn't be concerned about injection. Rather, you only have to worry about injection if 'columnname' is a tainted field. If it's untainted, then injection isn't a concern, so you can safely do the direct substitution. For example, if 'columnname' is coming from the URL, you definitely have to isolate it. But if it's coming from application code, you probably don't have to worry. cheers, barneyb ----- Excess quoted text cut - see Original Post for more ----- This is all to common even among experienced programmers. I recently caught a major case of this where the entire order by was passed via URL including exposing the table name as well as the column name in the URL. (domain.com?sortby=table.columnname desc) I like to use a cfswitch if there are more than two sort columns. I pass something like sort=name or sort=date etc in the URL. My table columns are never this simply named so I don't have any conflicts. Then in the cfswitch statement I have something like this <cfswitch expression"#url.sort#"> <cfcase value="name"> order by table.user_name <cfcase> <cfcase value="date"> order by table.lastlogin_date <cfcase> . . . </cfswitch> I NEVER user form or URL variables directly in the sql statement. I have a large number of validation functions that I use that will allow me to safely store any data into my db and to protect my applications from sql injection. cfqueryparam may do some of the work, but I do not rely on it completely. I also use my custom functions for data validation to help ensure I get the desired data input. -- Wil Genovese One man with courage makes a majority. -Andrew Jackson A fine is a tax for doing wrong. A tax is a fine for doing well. Barney Boisvert wrote: ----- Excess quoted text cut - see Original Post for more ----- Thanks Wil, That suggestion works. However, in an example of a query with 10 columns and assuming you want to allow desc and asc, does anyone have a more conscience way other than 20 cfcase statements? I thought about just checking for the existence of the semicolon in the sort variable being passed. If it exists, ignore it and default the sort. But...then I'd still be exposing the actual database field name which as you say, is a not desirable. I haven't attempted to use a framework yet. Do these already/automagically account for this? Does anyone else have another way of dealing with this? ----- Excess quoted text cut - see Original Post for more ----- Vince Collins wrote: > > That suggestion works. However, in an example of a query with 10 > columns and assuming you want to allow desc and asc, does anyone have a > more conscience way other than 20 cfcase statements? Check if your database support ordering by the ordinal number of the column in the resultset, i.e. the following sorts by t.B: SELECT t.A , t.B , t.C FROM table t ORDER BY 2 Jochem Jochem, good suggestion, then I can test for a numeric value. MS SQL Server does support this but then the application ends up being database specific and more cryptic to read. I could however live with that. Maybe I'm making this a bigger deal than it is. Just seems there should be a more elegant expression for this written by the much smarter than me CF-developers here :) Jochem van Dieten wrote: ----- Excess quoted text cut - see Original Post for more ----- I dont like using the order by 2 stuff simply because if the query has 42 columns, and it's 27 I'm looking for, it's a bitch to find.. it's easier to read "MONTHDATE" or whatever. Plus if the query changes, whoops.. gotta recount the items (unless its a small change of course) ----- Excess quoted text cut - see Original Post for more ----- I agree with you on this. Greg Morphis wrote: ----- Excess quoted text cut - see Original Post for more ----- Actually, I was thinking I could just test for the existence of a semicolon in the passed url variable, and if one exists, ignore it and default the sort but I'm not a SQL expert. My guess is that you can still do some ugly things after order by that doesn't require a semicolon Is this true? You could also pass the order by via URL, and that would cut the cfcases by half. ----- Excess quoted text cut - see Original Post for more ----- Yes, I did this in each case statement. <cfif url.sortby contains "DESC"> DESC</cfif> Also the expression for the cfswitch need to be modified to this <cfswitch expression="#ListFirst(url.sortby,',')#"> then I add that to the url sort variable like this. sortby=categoryname,DESC It's that simple -- Wil Genovese One man with courage makes a majority. -Andrew Jackson A fine is a tax for doing wrong. A tax is a fine for doing well. Vince Collins wrote: ----- Excess quoted text cut - see Original Post for more ----- > That suggestion works. However, in an example of a query with 10 > columns and assuming you want to allow desc and asc, does anyone have a > more conscience way other than 20 cfcase statements? You could specify a list of table names and use ListFindNoCase(). Or you could come up with an automatic solution by using JDBC metadata (or if/when you switch the CF8, the CFDBINFO tag) to get the valid column names for the table and compare the incoming order by value to those. You'd probably want to do this once per table and cache the column names somewhere so you wouldn't have to do it on every request. Not sure if your DB supports this but with Oracle I can select the columns from the table.. select column_name from user_tab_columns where table_name = 'MY_TABLE' Granted this would require 2 trips to the database, 1 to get the column names, then check to see if your sortby variable is in that list, then another to actually query the table. ----- Excess quoted text cut - see Original Post for more ----- Most RDBMSs support something like this. But you'd want to cache the results so you only have to do it once. ----- Excess quoted text cut - see Original Post for more ----- ----- Excess quoted text cut - see Original Post for more ----- I think this is how I would go about the new order by stuff . Instead of switch or if statements. run the query as one of the methods in a component, then all you need to do is pass an argumentcollection item over to handle the Order by portion. <cfset NewSort = component.method(qOrderField=Val(name)) /> <cffunction> <cfargument name="qOrderField" type="string" default="*"> <cfquery name="q"> select * from Order By #Argument.qOrderField# </cfquery> <cfreturn q> </cffunction> ----- Excess quoted text cut - see Original Post for more ----- How does that prevent SQL injection? Dave Watts, CTO, Fig Leaf Software http://www.figleaf.com/ Fig Leaf Software provides the highest caliber vendor-authorized instruction at our training centers in Washington DC, Atlanta, Chicago, Baltimore, Northern Virginia, or on-site at your location. Visit http://training.figleaf.com/ for more information! ----- Excess quoted text cut - see Original Post for more ----- Good point Dave, Guess you really need a list to start with or something... Still better than a bunch of if or switch statements. Test.cfm <cfset test = createObject("component", "test")> <cfset Thelist = "Col1,Col2,Col3,Col4"> <cfif IsDefined('URL.Sort')> <cfset sort = ListFindNoCase(Thelist, URL.Sort)> <cfset OrderByCol = ListGetAt(Thelist, sort)> <cfset NewSort = test.sort('#OrderByCol#') /> <cfdump var="#NewSort#"> </cfif> ~~~ Test.cfc <cfcomponent output="false"> <cffunction name="sort"> <cfargument name="qOrderField" type="String" default=""> <cfquery name="q" datasource=""> select * from table Order By #Arguments.qOrderField# </cfquery> <cfreturn q> </cffunction> </cfcomponent> Thanks everyone for your responses, In the end, it seems that combating the SQL Injection vulnerability when sorting is a matter of style. It's just important to note that cfqueryparam is your friend but not the only answer to the injection problem. Vince Collins http://www.vincentcollins.com > Actually, I was thinking I could just test for the existence > of a semicolon in the passed url variable, and if one exists, > ignore it and default the sort but I'm not a SQL expert. My > guess is that you can still do some ugly things after order > by that doesn't require a semicolon > > Is this true? There are SQL injection attack patterns that don't rely on the use of a semicolon, so you don't want to make that assumption. Dave Watts, CTO, Fig Leaf Software http://www.figleaf.com/ Fig Leaf Software provides the highest caliber vendor-authorized instruction at our training centers in Washington DC, Atlanta, Chicago, Baltimore, Northern Virginia, or on-site at your location. Visit http://training.figleaf.com/ for more information! I figured :) Dave Watts wrote: ----- Excess quoted text cut - see Original Post for more -----
|
Mailing Lists
|
Latest Fusion Authority Articles
|
||||||