Ben Nadel
On User Experience (UX) Design, JavaScript, ColdFusion, Node.js, Life, and Love.
I am the chief technical officer at InVision App, Inc - a prototyping and collaboration platform for designers, built by designers. I also rock out in JavaScript and ColdFusion 24x7.
Meanwhile on Twitter
Loading latest tweet...
Ben Nadel at Scotch On The Rock (SOTR) 2010 (London) with:

Learning ColdFusion 9: Using CFQuery In CFScript Can Enable SQL Injection Attacks

By Ben Nadel on
Tags: ColdFusion

Before I get into this, I just want to emphasize that this can only be a problem when you are using horrible SQL to begin with. However, I figured this was important to point out because newcomers to ColdFusion who start with ColdFusion 9 might not understand the way single quotes are auto-magically handled in a standard ColdFusion query statement. To demonstrate the potential SQL injection attack enabled by using CFScript-based queries, I am going to simulate a malicious form value:

  • <!---
  • Set a form variable. We are simply doing this to simulate
  • malicious code that might be submitted by a user as part
  • of a form submission.
  • --->
  • <cfset form.name = (
  • "Tricia' " &
  • "UNION ALL SELECT * FROM girl WHERE 'a' = 'a"
  • ) />

In this case, the application is expecting the user to enter a simple name value; but, the hacker on the other end is trying to inject additional SQL statements to the end of the form value in order to get extra records returned on the processing page.

Now, I'm going to try to use this malicious form value in a standard CFQuery tag:

  • <!--- Query for girls based on the form value. --->
  • <cfquery name="girl" result="getGirls">
  • SELECT
  • *
  • FROM
  • girl
  • WHERE
  • name = '#form.name#'
  • </cfquery>
  •  
  •  
  • <!--- Output girl. --->
  • <cfdump
  • var="#getGirls#"
  • label="Girl (via CFQuery / FORM.name)"
  • />

When we run this code, we get the following CFDump output:

 
 
 
 
 
 
ColdFusion Auto-Escapes Single Quotes Of Variables Evaluated Inside CFQuery Tags. 
 
 
 

As you can see in the output, ColdFusion auto-magically escapes single quotes contained within variables evaluated inside of a CFQuery tag. This is ColdFusion's way of preventing you from opening yourself up to a SQL injection attack. Rather than letting the malicious form value appending another SQL statement, the single-quote-escaping treats this entire phrase as the girl's name:

Tricia'' UNION ALL SELECT * FROM girl WHERE ''a'' = ''a

Now, let's jump over to using the new Query.cfc object inside of CFScript. In this attempt, we are going to be doing the same thing as above - putting the form value directly into the SQL statement without any query params:

  • <cfscript>
  •  
  • // Create a new query object.
  • getGirls = new Query();
  •  
  • // Set the SQL statement to gather the girls based on the
  • // form input. NOTE: We are NOT using query params on
  • // purpose to demonstrate a point.
  • getGirls.setSQL("
  • SELECT
  • *
  • FROM
  • girl
  • WHERE
  • name = '#form.name#'
  • ");
  •  
  • // Output girls query.
  • writeDump(
  • var = getGirls.execute().getPrefix(),
  • label = "Girl (via Query() / FORM.name)"
  • );
  •  
  • </cfscript>

When we run this code, we get the following CFDump output:

 
 
 
 
 
 
When Using CFQuery In CFScript As With ColdFusion 9's New Query.cfc, You Disable Auto-Escaping Of Single Quotes. 
 
 
 

Due to the way that ColdFusion compiles this query behind the scenes, it takes on an extremely different behavior. As you can see in the CFDump, the single quotes in our malicious form value are no longer escaped when they are evaluated; this allows the additional SQL statement to execute, becoming a SQL injection attack. This query, unlike the first one, now returns three records instead of zero.

Again, I want to emphasize that this query is poorly written to begin with and is really an outlier in terms of use-case; had we been properly using query params, none of this would be an issue. But, that said, I thought it was important to point out that by using CFScript-based queries, you are deactivating ColdFusion's auto-magic escaping of malicious SQL.

I don't consider this to be a bug in any way. In order to replace the query params into the original SQL statement, ColdFusion has to break the SQL statement into parts which it then writes inside of a CFQuery tag. In order for this to work, it must use the method, PreserveSingleQuotes(), such that valid uses of single quotes are not escaped auto-magically. I don't believe there is anything that they can do about this; so, at the end of the day, not a bug, just a word of caution.




Reader Comments

Awesome stuff Ben!! Good catch. You are absolutely right about it being bad form to start with, but I suspect that this is how 99% of developers start out. I know that I used CF for more than a year before I learned about cfqueryparam.

[quote]
by using CFScript-based queries, you are deactivating ColdFusion's auto-magic escaping of malicious SQL.

I don't consider this to be a bug in any way.
[/quote]

Well, it might have been done that way "by design", but I suspect the design could do with a second look.

As I would say most developers out there don't use parameterised SQL, this could be a bit of a problem. Then again that sort of developer is probably not going to be so inclined to use anything other than <cfquery> anyhow, I guess.

I think this should be brought to Adobe's attention.

--
Adam

Ben, is it OK if I reproduce sections of your original posting in a communication to Adobe? I'll attribute it appropriately, and cross-ref back to here.

--
Adam

Using cfqueryparam means that CF's db caching won't work on that query, so you'll have to roll your own caching, possibly storing the query in a app or session var and managing the timeout of the cache yourself. But anyone using cfscript for db queries will probably be doing that anyway.

@Gary

If you're talking about CACHEDWITHIN or CACHEDAFTER, CF8 lifted that restriction.

And if the choice is between <cfqueryparam> and CF's built in caching, roll your own caching scheme, or use one of the many solutions available.

Far easier than rolling your own SQL injection filter that's anywhere near as effective as <cfqueryparam>

@Adam and @Ben,

I think that using the statement "deactivating ColdFusion's auto-magic escaping of malicious SQL" was an unfortunate choice of words.

It's not that "magic quotes" are being disabled, it's that they they cannot be used.

By using cfquery within cfscript, you are essentially pre-contructing a string and inserting it into the cfquery object. You are then calling execute() on that object and the query is performed. At that point ColdFusion can no longer tell what was a query from a user-controlled scope and what was not. It is all one statement with the dynamic values already inserted.

I agree with Ben, this is definitely NOT a bug, and I doubt there is anything Adobe can do about it, even if brought to their attention. By all means, do so anyway if you still feel it is important, but I would suspect that if there was anything that could be done, that it would have been done by any of the other languages (Java, .NET, PHP, etc) that do queries this way and face the exact same issue.

For those that asked, yes it is possible to do parameterized quires with cfscript. I would suspect that Ben is working on a follow up post to demonstrate that. If not, then I will do one this weekend.

@Matt - thanks for the correction, I didn't realise the restriction was lifted. Good!

Having done some asp/vbscript sites I pass all user inputted strings through a server-side function to escape single quotes and remove security nasties. It made me *really* appreciate what cfquery[param] does automagically.

@Adam,

Yeah, of course bro, using anything you like.

@Jason,

Yeah, I don't think there is anything they can do. Perhaps, something that might be possible would be to add a boolean flag to the execute method:

execute( ignoreMaliciousSQL = true|false )

... this would default to false, and perhaps the execute() method could examine the SQL and throw an exception if it thought it might be malicious. Not sure if you can even tell things like this easily??

@Ben

I would think that it would be pretty difficult to determine if an fully constructed SQL statement was malicious at the time of execution.

For example

UPDATE users
SET active = 0
WHERE user_id = 1234;

DELETE
FROM user_history
WHERE user_id = 1234;

This could very easily be a dynamically constructed SQL statement that is inline with my business logic. It could also be a hacker who is trying to inject code to delete a user's history.

Besides, there already is an API for dealing with SQL injection in these queries, and that is by using query parameters.

In the case of your above query, you would replace '#form.name#' with a bind variable like :formName and then add a line like this above the execute() statement.

getGirls.addParam( name="formName", value=form.name, cfsqltype="CF_SQL_VARCHAR" );

Reference: http://www.aliaspooryorik.com/blog/index.cfm/e/posts.details/post/cfquery-in-cfml-with-parameters-218

Two comments:

1. I don't agree with this statement:
"As you can see in the output, ColdFusion auto-magically escapes single quotes contained within variables evaluated inside of a CFQuery tag. This is ColdFusion's way of preventing you from opening yourself up to a SQL injection attack." /Quote

I don't think that has anything to do with injection, I think it's just CF being nice and escaping quotes so when you do this:
x="That's really nice Tom";
Update table SET value='#x#';
You don't get a SQL error caused by the DB thinking you ended the string at "that'"

2. I would say the easiest way to add a flag to detect/prevent injection, is to test how many queries are being passed. Won't fix passing a UNION, but doing the old "');Drop Table Users" trick can easily be detected by compiling the SQL, and if the flag is true, it throws an error because two queries were passed not one. (At least that's how I'd do it.)

PS. http://xkcd.com/327/

@Matt

I"m not saying not to use cfqueryparam, in fact I think that is the best option, I was jut addressing Ben's Comment where he was saying:

Quote::
Perhaps, something that might be possible would be to add a boolean flag to the execute method:
execute( ignoreMaliciousSQL = true|false )

... this would default to false, and perhaps the execute() method could examine the SQL and throw an exception if it thought it might be malicious. Not sure if you can even tell things like this easily??
/Quote

And my thought was a sure way to tell if SQL is malicious is if it evaluates to two queries, when you're expecting only one.

@Matt,

No Problem, I realized after you challenged my answer that I should have been more clear on what problem I was solving. Because it's not a stretch to assuming I was speaking generally, and your response was absolutely correct.

Next time I'll be sure to make better reference to what I'm answering to avoid confusion.

Jason, good point re it being "too late" to escape any variable values by the time they get passed to the query "inner workings". One of the good things about tag-based stuff is that <cfquery> can automatically handle variables differently when they're embedded in its body. No such possibility with a function call.

So I think perhaps there's two things that could be done here:

1) have an escapeSingleQuotes() function which does the reverse of preserveSingleQuotes(), which one can use when building one's SQL string, eg:

sql = "select stuff from table where someString = '#escapeSingleQuotes(form.stringValue)#'";

Of course one needs to remember to use it. Not as elegant as having <cfquery> do it automatically.

2) rejig the way the query service does params, so they can be done inline in the SQL string:

sql = "select stuff from table where someString = #queryParam(value=form.stringValue, type='varchar')#";

The queryparam function simply returns a specially-formatted string, eg:

{queryparam value="" type="" etc}

which becomes part of the rest of the SQL string.

(it'd be more foolproof than that, to make it easier to match with a regex, and also less likely to conflict with anything else the SQL string is likely to have in it)

This would then be extracted by the internals of the string-parsing process, swapping the {queryparam} with a proper placeholder, and then binding the parameter value according to what the passed-in values were.

The reason I would prefer it to be done that way is I think it's more natural - for CF developers - to do the entire SQL expression in one hit, rather than write an SQL string and then add parameters separately. And making this more familiar might encourage people to parameterise heir queries more often.

I suppose the third thing to do is to specifically document the risks here, and strongly advise people to do their queries "properly". It's perhaps less likely to fall on deaf ears with the query service than one might expect, because the less "keen" (in two sense of that term) developers will probably be put off by all the dots and parentheses, and stick with <cfquery> anyhow.

From my perspective, if the behaviour, quirks and risks of something are documented properly, it's just "bad luck" if someone doesn't heed it, and gets bitten on the bum.

Finally, to Ben: I am not getting emailed when people post to your blog. Any reason?
--
Adam

It just dawned on me the power that could be used by keeping you SQL, and your params separate.. (Partially cause I ran into this recently.)

Having params like :Varname, mean you can have a central store for your SQL (either Object-wide, Application-wide, or.. dare I say, DB server specific..)

you could have a config file People.mySQL.SQL.cfm, to hold all the queries for that object, that are coded specifically for mysql. A beautiful way to keep DB interaction separate if needed.
(Pardon the sparse thoughts, I'm excited.)

I've always wanted to have a central place for my SQL, so that when there was a DB shift, or logic change, I could get at all the queries in one place. But the issue was always with how to store the params I was expecting like:

Select concat(firstname,' ',lastname) as PersonNamne from people where ID='#id#'

There's no good way to save the SQL in a form that I can pass in the ID later, with out some sort of replacement function that I really don't want to trust.
But with this, it is possible to just do something like:

PeopleQry.setSQL(DB.GetSQL(name='PersonQuery',DBtype='MySQL'));
PeopleQry.addParam(name="PersonID",value=ID,cfsqltype="CF_SQL_INTEGER");

Would have saved my butt a while ago, when I worked on a project that called for keeping all the SQL to certin tables to be centralized, and easier to read through then being spread out among functions.

@adam,

While i think that either of your suggestions would work, I don't think either of them are as elegant as simply adding params after the statement is constructed. It's really not that hard to get used to, and I have found as I have been working with these types of queries in Adobe AIR, it is not very difficult to create helper methods to be able to build your statements in one shot, if you want. It might look something like this.

sqlHelper.insert(table="users" ,columns="username,firstname,lastname", values=[form.username,form.firstname,form.lastname]);

Here I am passing in the table name, the names of the columns and then the values. The helper function would then be able to construct a properly parameterized SQL statement and then execute it without much thought from the developer.

I have not tried this yet with CF9, but I am doing it with JS and AIR and it works pretty slick.

Hi Jason
You don't have to convince /me/, I'm absolutely fine with doing it as per the current approach.

I'm just concerned about buy-in from the sort of people who don't currently parameterise their queries, and are likely to fall foul of this issue Ben's noticed. Anything to make their migration to "doing it properly" easier.

Tim:
I think you could benefit on looking @ Reactor or TransferORM or, indeed, the Hibernate support added to CF9. Or just reading about the concepts of n-tier development.

--
Adam

Great conversation; I just wanted to say that I am not recommending that people don't use Query Params in any way - they are awesome and should be used. I don't even think we need to talk about that. All I was demonstrating was that for people who are not using them to being with, as @Adam points out, can be open up to SQL attacks.

@Tim's idea though, of counting the number of queries could be good I think.

@Adam,

I had looked at transfer, and actually already build a much more simplified DB handler before I'd heard of it. I was just commenting on how for complex queries in a past project this would have been a perfect fit, and may still be for those who don't want to wrap their head around some of that.

@Ben,

Thanks.

Why adobe would give you script equivalent of cfquery is beyond me. I love cfquery tag because it helps me wriite clean sql, and get away from the horrible jdbc queries

If I wanted to write javalike code I would have written java

anang--

I imagine that a cfscript version of cfquery was implemented because people wanted it and asked for it.

I personally love it--while it is a bit more work than the tag based version, it fits nicely within the already-heavy cfscript work that I do, preventing me from having to break out of cfscript just to run a query.

Plus, without a cfscript based version of cfquery, cfscript-based components would probably be pretty worthless...

Funny thing is no-one here has talked about a global sql injection validation.

Even if you have the worst sql in the world, you should be checking and validating every url and form variable that passes into your application for malicious content.

You should also separate your datasources into ones that are read-only and writeable so that basic select queries can never modify data even with injection as they are using read-only. CREATE and DROP functionality should only be done with admin level datasources.

And where possible you should use stored procedures/functions to do more advanced queries, where once again the data can be validated.

@Dawesi,

ColdFusion does have *some* built-in Cross-Site-Scripting (XSS) attack prevention for URL and FORM variables; but from what I have heard, it's not that great. I believe that Pete Freitag's FuseGuard application Firewall (http://foundeo.com/security/) performs much more indepth checking.

That's pretty interesting regarding the two types of data sources, however. I had not considered creating Read-Only and and Full-Access one. That's kind of a cool concept though! Thanks for bringing that up.

@Ben,

I was referring to 'writing your own' global sql injection validation and/or using Pete Freitag's protection.

I'm surprised this kind of protection isn't built into most of the frameworks out there as an option.

Another common problem for PCI compliance is making sure 'note' fields don't have credit cards in them, so this global filtering would allow you to do this also, using the same engine.

---

Another way of protecting the core system is to have multi-tiered application eg:

L1) Admin/Public/Api layer for intefacing the system

L2) An (optional) broker for queuing, re-validating and parsing the requests from 2) to 1) above.

L3) An core api layer that holds the data and runs the system

@Dawesi,

I don't have too much framework experience, so I can't vouch for what's in them. I would be surprised if ColdBox didn't have *something* in it - Luis seems to think of every possible angle of application development :D

Scrubbing out passwords and credit cards is one of those things that I had never heard of before; then I saw it in a security presentation and it was very jarring! Most of the time it's not a problem - the thing where I have to keep remembering to scrub is in the error logging. If a requests causes an error, I typically log the URL/FORM scopes; but, as you say, I have to be very careful to not log people's CC numbers or passwords. Great comments!

I recently learned that there are some cases where automagic quote-doubling doesn't happen in cfquery. I was writing code to do bulk data import. When writing a single query that inserts 500 rows at a time, I found that there is a limit to how many cfqueryparam tags you can have in a single query. So I was relying on automagic quote handling for string values.

Eventually, a bug turned up if you have a string value formatted as a database timestamp. For example, "{ts '2011-01-01 12:00:00'}". In this case, CF does not escape the single quotes, and you get a query error.

Example

  • <cfset str1 = "kip's string" />
  • <cfset str2 = "{ts '2011-08-26 15:21:09'}" />
  • <!--- The following query executes correctly --->
  • <cfquery name="tst1" datasource="MyDS">
  • SELECT * FROM MyTable WHERE MyField = '#str1#'
  • </cfquery>
  • <!--- The following query FAILS because single quotes in str2 are not escaped. --->
  • <cfquery name="tst2" datasource="MyDS">
  • SELECT * FROM MyTable WHERE MyField = '#str2#'
  • </cfquery>

I don't know if there are any other types of strings which exhibit this behavior, and I couldn't find this discussed anywhere after some Googling. I'm also not sure if this is a CF bug or a feature.