Ask Ben: Protecting Database Table Names In ColdFusion CFQuery
Posted July 26, 2007 at 2:33 PM by Ben Nadel
This may be to elementary for an "Ask Ben" but I cannot find information anywhere on how to protect a query when a table name is passed in via a form or url. For Example:
<cfquery name="xyz" datasource="ds123">
UPDATE #form.table#
SET active = <cfqueryparam cfsqltype="cf_sql_bit" value="1">
</cfquery>
I cannot put the table name in a cfqueryparam, I tried, wasn't sure if it would work or not. Am I stuck making a quick function to verify that value is a single word and if its not return some bogus table name? Thanks for any help.
Let me start off by saying that, while I do not know what your exact situation is, this is something that makes me very nervous! If this is for your own personal use, that is one thing, but something like this should NEVER be available in any sort of public capacity as it will leave you open to different types of malicious behavior! Red Flag! Red Flag!
That being said, in order to make sure that the table is valid, we just have to check it against a set of explicit table names. This is the best way to make sure that only valid table names can ever be entered. If anyone ever tries to mess with the FORM-based table name, at least the ColdFusion CFQuery tag will never execute. Furthermore, before we even examine the table name, let's strip out as many invalid characters as we can.
- <!--- Param the table name in the FORM scope. --->
- <cfparam
- name="FORM.table"
- type="string"
- default=""
- />
-
-
- <!---
- Before we even check anything, let's clean characters
- out of the table name that we KNOW cannot be valid.
- Things like single quotes and semi colons might be a
- sign of a SQL injection attack. Let's strip out anything
- that is not standard.
- --->
- <cfset FORM.table = FORM.table.ReplaceAll(
- "[^\w\-_]+",
- ""
- ) />
-
- <!---
- Passing in a database Table as a variable is VERY
- risky because it leaves you open to all kinds of
- malicious attacks and tomfoolery. Therefore, we
- have to be ultra careful about how we handle this.
- Make sure that the table is a valid table and
- only allow table names that are in the following list.
- --->
- <cfsavecontent variable="strTableList">
- address
- contact
- phone
- notes
- </cfsavecontent>
-
- <!---
- Check to see if the table is valid. Tread the space,
- tab, line break, and carriage return all as list
- delimiters - this just makes the list easier to read.
- Also, do NOT use NOCASE search. Let proper name
- case be yet another layer of security.
- --->
- <cfif ListFind(
- strTableList,
- FORM.table,
- " #Chr( 9 )##Chr( 13 )##Chr( 10 )#"
- )>
-
- <!---
- Get the record count for the given table. When
- putting in the table name, be sure to use the []
- notation so that we do not get any invalid table
- name exceptions.
- --->
- <cfquery name="qTable" datasource="#REQUEST.DSN.Source#">
- SELECT
- COUNT( * ) AS count
- FROM
- [#FORM.table#]
- </cfquery>
-
-
- <!--- Output record count. --->
- <p>
- Records in #FORM.table#: #qTable.count#
- </p>
-
- <cfelse>
-
- <!---
- A valid table name was NOT found. Throw an error
- or handle in some other way.
- --->
- <cfthrow
- type="TableNotFoundException"
- message="The table [#FORM.table#] is not valid"
- />
-
- </cfif>
Give that a go... and again, I would recommend this only for personal use.
Reader Comments
I'm not sure how you go about it with other DBMS, but if you are using MS SQL you have the ability to query the DB for a list of the tablenames:
SELECT name
FROM dbo.sysobjects
WHERE (xtype = 'U')
Each rdbms has its own way of storing and giving you access to its database metadata. However, you could achieve the same above with dynamic table names using CF8's new cfdbinfo tag.
I was thinking about bringing up some sort of introspection, but I just felt that explicitly listing out "valid" tables is the safest thing to do (that, and I have never done any kind of introspection in that way, so I didn't want to give misinformation on it). Plus, with explicit name, you don't have to allow access to all DB tables, just a select set - this might help to minimize any damage that someone might be trying to cause.
You could also:
- create an array or structure mapping the table names
- have a drop down with the array number or key as a value
- then when submitted if the submitted value is not in the array or structure you know you have a problem!
cfdbinfo will also help with this process in the future.
Ben,
Good post. That is very true that if we specify explicit names of tables it is the safest. Let us see with real example that SQL can do the table exist check how application help database with the task.
We are using application where we create lots of dynamically created tables, all are real tables and all are dropped when day long running processes is over.
We use frequent check in our system to make sure that table exist (and safe).
IF EXISTS(SELECT name
FROM sys.tables
WHERE name = 'yourtablename')
--Processes goes here.
Interesting that we have all the table with specific prefix and we check in ColdFusion using list function listfindnocase . If this CF check fails, we even do not go to query part.
Regards,
Pinal
This maybe a bit of dirty code: (but it cannot be any dirtier than allready described by the requestor)
Why not make a list of "safe" to edit tables and check with listFindNoCase(application.safeToEditTableList,form.tablename) or do a cfswitch with the tableaname?
Ben,
You make a good point about what to do to enforce validity, but I disagree that you should /never/ do it for customer use.
I think variable table names are a good way to avoid duplication in your code in some situations, or to avoid long logic chains.
@Sam,
While I agree that variable names is a good thing and can be used to avoid duplicate effort and all that goodness, I guess what rubbed me the wrong way was the fact that the table name was passed via the URL / FORM scopes. This just feels so wrong (and not in the good way). Of course, I don't know the context it is being used, so I cannot say for sure; however, you have to take into account things like people hacking the FORM and changing value (something that FireFox makes SOOO easy).
Context is very important, but it still rubs me the wrong way.
Well, I was thinking about passing it through the form. For instance, if you need to do a report from one of several different tables, you might allow them a select form field with table names as values.
I haven't actually used that, but I have allowed them to select column names to report on. Of course, I don't do much work on public websites where this would be a problem - I do mostly backend work for specific clients where the general public doesn't have access, so my perspective is different.
In the case where you do find it easier to allow table and column names, it is good to check them against specific values - if anything you can report an error message without doing try/catch or worse, just letting the unfriendly error message show up to the user.
@Sam,
Even in a case where you let people select table columns for a report, I wouldn't inject the table name directly into the SQL statement. Instead, I would use some logic around the column name. For example, instead of just doing something like this:
SELECT #column_list# FROM [table]
... I would rather do something like this:
SELECT
<cfif ListFind( "id", column_list )>id,</cfif>
<cfif ListFind( "name", column_list)>name,</cfif>
.....
FROM [table]
This way, you can still let the user select random columns, but you enforce the column choices by checking their existence in logic, rather than just dumping output. Yes, my way does require more logic, but I just feel better about it. Plus, there is usually SQL optimization that you can do around the table columns - for instance, the absence of a particular column name might mean you don't have to perform a particular join.
