Inspiring

Question

Sanitize a Query with Dynamic Table Name

Forum|Forum|7 years ago
September 7, 2018
3 replies
1251 views

Hi all,

I inherited an application and am stumped on sanitizing a dynamic query that is being flagged on a vulnerability scan.

It starts with a CFINVOKE that passes parameters including tablename, column_names, conditionals.

The function invoked then executes the query something to this effect...

<cfquery name="updateMe" datasource="#myDBsource#">

UPDATE #arguments.tablename#

SET ...

WHERE #arguments.conditionals#

</cfquery>

Usually I would use the <cfqueryparam value="#arguments.tablename#" cfsqltype="CF_SQL_VARCHAR" maxlength="17"> in the conditional, but not sure how to handle when the dynamic part is the actual table name, column name, or conditionals.

Thanks in advance for any help.

This topic has been closed for replies.

BKBK

Community Expert

You should heed what WolfShade says. A major advantage of cfqueryparam is security. However, even if it is possible to pass user-input in this way to your query, you will be undermining security. That is because you're no longer using cfqueryparam to sanitize column values.

By allowing whole statements to enter your query dynamically, you're inviting trouble. An attacker may craft a trojan.

That said, you could solve your puzzle as follows, applying cfqueryparam to the field value, as intended.

UPDATE #arguments.tablename#

SET ...

WHERE #listFirst(arguments.conditionals, "=")# = <cfqueryparam value="#listLast(arguments.conditionals, '=')#" cfsqltype="CF_SQL_VARCHAR">

(if the value is an integer, then you should of course use cfsqltype="CF_SQL_INTEGER")

D

Dave Watts

Community Expert

Assuming you have a fairly short list of items, you could use conditional logic to add a static clause to your SQL.

Dave Watts, Fig Leaf Software

Dave Watts, Eidolon LLC

WolfShade

Legend

I respectfully disagree. Having one dynamic query to select/update/insert/delete (let's say) 20 different functions is insecure, and even if you throw conditionals in there with static results you're still using one query for 20 different functions but turning it into spaghetti code, potentially making it more difficult to support if there are more functions added to it down the line. Granted, that's just my opinion, but it could soon become untenable. Just a bad practice, all around.

If this query is supporting >100 processes, it is going to take time but the OP really should write distinct queries for each. In the end, it will be easier to maintain, more secure (be sure to use canonicalize() before cfqueryparam), and probably a lot more efficient.

Just my two cents.

V/r,

^ _ ^

D

Dave Watts

Community Expert

I agree that using queries with dynamic SQL code (as opposed to data) is not a good idea. But it can be done safely, in the strict sense that you're not actually dynamically writing the SQL code itself, just using variables to figure out which branch of SQL code you want to use in your query. I also agree that this is not really optimal and is almost always a bad practice. Another thing you didn't mention is performance - queries with dynamic SQL will not have useful prepared statements and will likely run slower. So I agree with your respectful disagreement!

Dave Watts, Fig Leaf Software

Dave Watts, Eidolon LLC

WolfShade

Legend

AKAIK, there is no safe way to execute a dynamic query. They should be avoided. This method reeks of laziness. Create individual queries for each need.

I hate to sound trite, and I know it isn't the answer you want to hear, but this really should be avoided.

V/r,

^ _ ^

Sign up

To post, reply, or follow discussions, please sign in with your Adobe ID.

Sign in to Adobe Community

To post, reply, or follow discussions, please sign in with your Adobe ID.

Scanning file for viruses.

This file cannot be downloaded