Dynamic SQL - sanitize without CFQUERYPARAM?

Report · Sep 19, 2012

Hello, everyone.

I'm trying to build a dynamic WHERE clause for a query, based upon a user form submit. Since I can't use CFQUERYPARAM for the whole WHERE clause, is there a way to sanitize the WHERE clause?

I cannot use Stored Procedures, and it's an Oracle database, not my familiar MS-SQL.

Thank you,

^_^

Report · Sep 19, 2012

The main thing you have to do is escape all single quotes within string values and make sure number values are numbers by using val(), especially if the values come from the client. Lastly, you'll need to use preserveSingleQuotes(queryStr) when you do finally use your dynamic query string.

Report · Sep 19, 2012

Thank you for the reply.

I have PreserveSingleQuotes in place, already. I am just worried about removing SQL commands from the user input.

The query is to search for Training Programs, and the user can enter free text into input type="text" fields for searching against program name, program description, and program type.

I do have JavaScript in place to remove all punctuation (including the semi-colon and apostrophe), and the database does not allow chained commands. But I do not feel this is secure enough.

Currently, based upon user input, the where clause is being pieced together based upon conditionals before the query is executed, so my current query looks like (pseudo code):

SELECT colA,.. colF

FROM table

#PreserveSingleQuotes(whereClause)#

ORDER BY colA, colD

What is the best way to prevent SQLi since I cannot use cfqueryparam?

Thank you,

^_^

Report · Sep 19, 2012

Like I said, escape single quotes and use val() for numeric values from the client. NEVER allow stuff like this in your where clause: where someString='#URL.someString#' or someNumber=#FORM.someNumber# . This must be coded like: where someString='#replace(URL.someString,"'","''","ALL")#' or someNumber=#val(FORM.someNumber)#. Really, you probably need more validation, but this is the minimum.

But like Jason, I would lean toward redesigning to be able to use the cfqueryparam. It's a lot more idiot proof which will come in handy when some other programmer updates your work -- or you update your own work a few years from now and forget or overlook the gotchas.

Report · Sep 19, 2012

You could use cfqueryparam if you pieced together the WHERE clause INSIDE of your cfquery. That is a much better practice overall.

SELECT colA,.. colF

FROM table

WHERE 1=1

AND colA = <cfqueryparam value="#FORM.colA#" cfsqltype="cf_sql_varchar" />

</cfif>

AND colB = <cfqueryparam value="#FORM.colB#" cfsqltype="cf_sql_varchar" />

</cfif>

ORDER BY colA, colD

</cfquery>

Report · Sep 19, 2012

I really dislike throwing conditionals inside a cfquery, but I think Jason's suggestion is the way to go.

Thanks to everyone!

^_^

Report · Sep 19, 2012

WolfShade,

How else would you do it? How do you create dynamic SQL statements?

Jason

Report · Sep 19, 2012

Sorry for taking so long to respond; I was away at lunch. Jimmy John's, yum.

</cfif>

</cfif>

SELECT colA, colB

FROM tableA

#PreserveSingleQuotes(var)#

ORDER BY colB

</cfquery>

^_^

Report · Sep 19, 2012

I guess it must be personal preference. Because I could not tolerate workign like that. No offense intended.

I am hoping that you just left out the part where you sanitize the input for those input params, because otherwise that code opens you up to even more SQLi than normal. PreserveSingleQuotes() is dangerous to use in SQL if you don't know what you're doing.

Jason

Report · Sep 19, 2012

Since the conditionals are now inside the cfquery tags, there's no need for PreserveSingleQuotes. I'm letting the cfqueryparam do what it's supposed to do.

^_^

Report · Sep 19, 2012

WolfShade wrote:
I really dislike throwing conditionals inside a cfquery,

Why's that?

--

Adam

Report · Sep 19, 2012

I also dislike having conditionals inside a query. I think it spagettifies the code. On a page that accepts user inputs, uses them in a query, and outputs something, I like this sort of stucture.

1. Validate user inputs

2. Process user inputs and anything else that needs to be done.

3. Queries.

4. Output

Having said that, I wouldn't do this: "I do have JavaScript in place to remove all punctuation (including the semi-colon and apostrophe),". Punctuation marks could be valid characters in the search string, especially if you are doing a search by people's names.

Report · Sep 19, 2012

Again I ask, how do you do a dynamic query then? How do you conditionally include pieces of a where clause?

jason

Report · Sep 19, 2012

How do I do a dynamic query? It's hard to say because it's been years since I've had to do one. I was less of a fan of query parmaters back then because I did almost all my work with a redbrick database and the driver we had at the time did not support them. Either that or I was doing something wrong that caused errors.

Having said that, I simply do what I think is best for the situation at hand. While I recognize the value of query paramters, I also recognize the value of simple, maintanable code. There is a limit to the amount of complexity I will introduce when it's only purpose is to use query parameters.

Report · Sep 19, 2012

Also, SQL injection protection is not the only benefit of using <cfqueryparam>. By parameterizing queries, the RDBMS can reuse query plans effectively when processing each <cfquery> call.

Let's say this same CFM page is called over and over with different values in the where clause. If you don't use <cfqueryparam>, the RDBMS will have to calculate the query plan every time the page is called, which will overall can increase the time taken to return results to ColdFusion. If you use <cfqueryparam> however, the RDBMS will attempt to reuse the query plan from the last time the page was called (as long as the SQL query structure is the same), and will likely perform the query and return the results faster. Even if you have a query with conditionally included parameters (like Jason suggests), each unique combination of included parameters will result in a distinct query plan, and the next time that same combination of included parameters is passed that specific query plan will likely be reused, which still saves RDBMS processing time.

-Carl V.

Report · Sep 19, 2012

Regarding, "Also, SQL injection protection is not the only benefit of using <cfqueryparam>. "

I believe that query parameters have many benefits, but that is not one of them. This opinion is based on my observations.

A few years ago I did some tests using oracle, sql server, and redbrick databases to see when unwanted sql might run. The only time I could get it to run was with sql server numeric datatypes. In all other cases, the sql was simply part of the string, or the query would crash.

At the same time I tested javascipt injection. With or without query parameters, the script would be stored to the db normally, and if I retrieved it and outputted to a browser, it would execute.

My conclusion is that query params and security plans are both good things, but they are unrelated.

Report · Sep 19, 2012

Dan Bracuk wrote:
Regarding, "Also, SQL injection protection is not the only benefit of using <cfqueryparam>. "
I believe that query parameters have many benefits, but that is not one of them. This opinion is based on my observations.
A few years ago I did some tests using oracle, sql server, and redbrick databases ...

And no doubt you ran every possible SQLi attack vector </sarcasm>

It is certainly true that "magic quotes" will stop some SQLi, but it will not stop all. And it certainly won't stop it if preserveSingleQuotes() is used.

Whether or not you believe that cfqueryparam helps with SQLi is immaterial. It does. And it is more effective than any other method of protection.

At the same time I tested javascipt injection. With or without query parameters, the script would be stored to the db normally, and if I retrieved it and outputted to a browser, it would execute.

That is a distinctly different issue that you've now brought into this and it points out the importance of outputting untrusted input using the proper encoding method. It is not SQLi injection and the fact that it was allowed while using <cfqueryparam /> does not negate <cfqueryparam>'s effectiveness against SQL injection.

<cfqueryparam /> is related to security (even if only a side-effect of their original purpose) and should be employed to prevent SQLi.

jason

Report · Sep 19, 2012

Regarding, "And it certainly won't stop it if preserveSingleQuotes() is used."

I tried that, and disagree.

Report · Sep 19, 2012

Dan Bracuk wrote:
Regarding, "And it certainly won't stop it if preserveSingleQuotes() is used."
I tried that, and disagree.

Then you did it wrong.

preserveSingleQuotes() will prevent magic quote from being placed which, when done properly, will allow SQLi statements to pass. The fact that you were unable to successfully exploit a DB in your attempts does not mean it's impossible. I have done it.

I sincerely doubt that your attempts at SQLi attack were comprehensive. Unless you tried every possible attack vector you cannot validly conclude that cfqueryparam is ineffective for SQLi protection. And since you have already displayed a misunderstanding as to what SQLi actually is, I hope no one will listen to you.

jason

Report · Sep 19, 2012

Dan,

I don't know where 'security plans' was mentioned in my post.

My conclusion is that query params and security plans are both good things, but they are unrelated.

I'll stand by my SQL injection protection statement though. When you use <cfqueryparam> and include the CFSQLType attribute in the tag, you not only tell your RDBMS what the data type of the parameter is, but if it is a string, it is automatically wrapped in single quotes. This will prevent any embedded SQL statements from being parsed and executed by the RDBMS during the execution of the query. This is backed up by the Adobe Livedocs section on <cfqueryparam>. Utilizing <cfqueryparam> is Adobe's recommendation, as stated at the top of that document. Here is an excerpt:

"Adobe recommends that you use the cfqueryparam tag within every cfquery tag, to help secure your databases from unauthorized users. For more information, see Security Bulletin ASB99-04, “Multiple SQL Statements in Dynamic Queries,” at www.adobe.com/go/sn_asb99-04, and Accessing and Retrieving Data in the Developing ColdFusion Applications."

I'll also stand by the explained behavior of query plans, as this too is backed up by the same Livedoc. It refers to this functionality as "SQL bind parameters", and explains it this way:

"To benefit from the enhanced performance of bind variables, use cfqueryparam for all ColdFusion variables, and your DBMS must support bind variables. If a DBMS does not support bind parameters, ColdFusion validates and substitutes the validated parameter value back into the string. If validation fails, it returns an error message."

Looks like Jason beat me to this response while I was busy verifying the Livedocs!

-Carl V.

Report · Sep 19, 2012

Having said that, I wouldn't do this: "I do have JavaScript in place to remove all punctuation (including the semi-colon and apostrophe),". Punctuation marks could be valid characters in the search string, especially if you are doing a search by people's names.

Nor would I ever rely on JavaScript to do this sort of thing. JavaScript can be bypassed easily enough. Validation / cleansing like this needs to be done on the server, not on the client.

And yeah, there'd be plenty of people with Irish background would might not like having the apostrophe taken out of their surnames... ;-). I'm pretty sure no-one's gonna have a semi-colon in their name though, but your point is a sound one: don't mess with search strings: let the person type in whatever they want, and just deal with it properly.

--

Adam

Report · Sep 19, 2012

The search is against Program Names, Program Types, and Program Description, none of which contain apostrophes.

^_^

Report · Sep 19, 2012

As Dan stated, it "spaghettifies" the code, makes it ugly and sometimes extremely difficult to read. Just a personal preference. I'm a little OCD about certain things.

^_^

Report · Sep 19, 2012

As Dan stated, it "spaghettifies" the code, makes it ugly and sometimes extremely difficult to read. Just a personal preference. I'm a little OCD about certain things.

I'm not with either of you on this one.

The precise place one should be organising one's SQL string to send to the DB is between the <cfquery> tags. It's the whole reason that <cfquery> is like this:

Stuff to create SQL string here

</cfquery>

And not:

Stuff to create SQL string here

By decoupling where you manage the SQL string and where you send it to the DB you are adding convolution, not reducing it.

--

Adam

Report · Sep 19, 2012

Or if you were to use Query.cfc instead, you could put parameter "markers" into your SQL string, eg:

"WHERE foo = ?" or "WHERE foo = :fooParam"

Rather than needing to use the <cfqueryparam> tag at all.

--

Adam

Adobe Community

Dynamic SQL - sanitize without CFQUERYPARAM?

1 Correct answer