Skip to main content
P
pham_mn
Known Participant
August 14, 2019
Question

Avoid dynamically constructing SQL queries

  • August 14, 2019
  • 3 replies
  • 825 views

Hello all,

We are scanning the CF files using Veracode software, and after scanning: Veracode returns some of the files that need to be address the

dynamically constructing SQL queries. The below codes is dynamically constructing SQL queries.

<cfquery name="qBprfp490" datasource="Hobbes">

SELECT mprno

FROM #aPacket[1]#.bprfp490

WHERE mprnoa = <cfqueryparam value="#aValue#" cfsqltype="cf_sql_varchar" >

</cfquery>

I think by placing aValue inside the <cfqueryparam> tag will resolve the SQL injection. But I think it requires me to fix the line that starts with the word "FROM"

The below is the instructions from Veracode regarding how to avoid dynamically constructing SQL queries

"Avoid dynamically constructing SQL queries. Instead, use parameterized prepared statements to prevent the

database from interpreting the contents of bind variables as part of the query. Always validate untrusted input to

ensure that it conforms to the expected format, using centralized data validation routines when possible"

Any advise how to fix dynamically constructing SQL queries please

Thanks

HP.

pham_mn@yahoo.com

This topic has been closed for replies.

3 replies

D
Community Expert
Dave WattsCommunity Expert
Community Expert
August 15, 2019

If the variable doesn't come from the user, or isn't manipulated by the user, you can safely disregard this message. This is true for any variables within SQL queries. The problem is, most variables do either come from the user or are manipulated by the user in some way.

You could use CFQUERYPARAM here, I think, but Wolfshade's answer is really better I think. Presumably, you have a fairly short list of schemas. You could use CFIF, or probably better yet CFSWITCH, to match the user's entry with a schema, then hard-code the schema within the conditional branch. This would let you avoid the possibility of a mismatched entry coming from somewhere unexpected, and would avoid the potential downside of parameterizing your schema value. This potential downside is small, but does exist. If you have a short list of possible values, it can be better to explicitly specify them within your SQL code rather than within data values.

Dave Watts, Eidolon LLC

Dave Watts, Eidolon LLC
WolfShade
WolfShade
Legend
August 15, 2019

Precisely what I was thinking, Dave.  Something before the query, like:

<cfswitch expression="#aPacket[1]#">
     <cfcase delimiter="," value="validSchema1,validSchema2,validSchema3"></cfcase>
     <cfdefaultcase><cfabort /> or other code, depending how you prefer</cfdefaultcase>
</cfswitch>
<cfquery name="qBprfp490" datasource="Hobbes">
SELECT mprno
FROM #aPacket[1]#.bprfp490 
WHERE mprnoa = <cfqueryparam value="#aValue#" cfsqltype="cf_sql_varchar" >
</cfquery>

Or, within the query, I suppose, but that could be a bit confusing:

<cfquery name="qBprfp490" datasource="Hobbes">
     SELECT mprno
<cfswitch expression="#aPacket[1]#">
     <cfcase value="validSchema1">
          FROM validSchema1.bprfp490 
     </cfcase>
     <cfcase value="validSchema2">
          FROM validSchema2.bprfp490 
     </cfcase>
</cfswitch>
     WHERE mprnoa = <cfqueryparam value="#aValue#" cfsqltype="cf_sql_varchar" >
</cfquery>

But then I'm not sure how I'd abort or return a message in this scenario.

If this is within a cfc function, you could return an error message instead of the abort, shutting down the function before it gets to the query.

Just my thought.

V/r,

^ _ ^

P
pham_mnAuthor
Known Participant
August 15, 2019

Hello all,

Thank you for all your ideas/thoughts.  I will take your ideas to change the code and scan again to see if Veracode  still doesn't like it.  I will keep you posted

Again...THANK YOU!!!

WolfShade
WolfShade
Legend
August 15, 2019

I have to say, it is dangerous enough to use dynamic column names or table names, but you want to use a dynamic schema name.

There is all kinds of advice in a Google search about how to parameterize a table or column name.  But, personally, I'd rather gatekeep something like this with a whitelist.  If a supplied schema name doesn't match valid names, abort.  If it does, go ahead.

V/r,

^ _ ^

    Chris Bossons
    Chris Bossons
    Participant
    August 15, 2019

    Hi,

    I think the line it has an issue with is #aPacket[1]#.bprfp490

    I assume #aPacket[1]# defines the schema the table lives in, but because this is referenced by a variable, this is still injectable if that variable can be hijacked.

    If possible, hard coding the reference to the schema should resolve errors being thrown.

    Terms & ConditionsAccessibility statement
    Using the community Terms of use Privacy Cookie preferences Do not sell or share my personal information ad choicesAdChoices