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

Posted August 6, 2009 at 8:26 PM

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:

 Launch code in new window » Download code as text file »

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:

 Launch code in new window » Download code as text file »

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

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:

 Launch code in new window » Download code as text file »

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

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.

Download Code Snippet ZIP File

Post Comment  |  Ask Ben  |  Print Page

• Add To Del.icio.us
• Add To Digg
• Add to Spurl.net
• Add to Wists
• Add to Simpy
• Add to Newsvine
• Add to BlinkList
• Add to Furl
• Add to reddit
• Add to FARK
• Add to BlogMarks
• Add to Yahoo! My Web
• Add to smarking
• Add to Ma.gnolia
• Add to Segnalo

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.

So, can you use <cfqueryparam> in cfscript in CF9? I use it exclusively when writing queries in CF7 and CF8...

Good catch and good to know! Thanks, Ben.

[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

@Jason,

Yeah, it probably would be hard to figure out. I want to put an idea down though. I post back later.

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/

@Tim

Seems to me that's a lot of effort that could be easily avoided by using <cfqueryparam>

@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.

@Tim

Sorry- I managed to completely miss your point. Seems to be my theme for the day. :(

@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

Post Comment  |  Ask Ben

Post A Comment

Comment Etiquette: Please do not post spam. Please keep the comments on-topic. Please do not post unrelated questions or large chunks of code. And, above all, please be nice to each other - we're trying to have a good conversation here.

Please review the following issues:

Author Name:
Author Email:
Author Website:
Comment:	Formatting: <strong>bold</strong> <em>italic<em>
	Remember my information
	Subscribe to entry *
	Send me a copy of this comment (as part of the comment thread)

 

* If you subscribe, any new posts to this thread will be sent to your email address.