• Global community
    • Language:
      • Deutsch
      • English
      • Español
      • Français
      • Português
  • 日本語コミュニティ
    Dedicated community for Japanese speakers
  • 한국 커뮤니티
    Dedicated community for Korean speakers
Exit
0

Avoid dynamically constructing SQL queries

Community Beginner ,
Aug 14, 2019 Aug 14, 2019

Copy link to clipboard

Copied

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

Views

542

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
New Here ,
Aug 15, 2019 Aug 15, 2019

Copy link to clipboard

Copied

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.

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Aug 15, 2019 Aug 15, 2019

Copy link to clipboard

Copied

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,

^ _ ^

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Expert ,
Aug 15, 2019 Aug 15, 2019

Copy link to clipboard

Copied

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

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Aug 15, 2019 Aug 15, 2019

Copy link to clipboard

Copied

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,

^ _ ^

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Beginner ,
Aug 15, 2019 Aug 15, 2019

Copy link to clipboard

Copied

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!!!

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Community Beginner ,
Aug 15, 2019 Aug 15, 2019

Copy link to clipboard

Copied

This is the entire codes look like

<cfsetting showdebugoutput="No">

<cfset aPacket = DeserializeJSON(JSONContent)>

<cftry>

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

          SELECT mprno

          FROM #aPacket[1]#.bprfp490

     WHERE mprnoa LIKE <cfqueryparam value="%#aPacket[2]#%" cfsqltype="cf_sql_varchar" >

     </cfquery>

<cfcatch type="database">

     <cf_frmErrMsg Type="#cfcatch.type#" Message="#cfcatch.message#" Detail="#cfcatch.detail#">

     <cfabort>

</cfcatch>

</cftry>

Any thought?

Thanks

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
LEGEND ,
Aug 16, 2019 Aug 16, 2019

Copy link to clipboard

Copied

LATEST

You're not doing anything to gatekeep or parameterize the schema name.  Which if the schema name is not modifiable from a user may not be bad, but it's still not really safe.  Since you're converting JSON, I'm going to assume that the value is in plain text as it is being passed to the function.

See my most recent reply before this one.  The first example uses a switch/case statement to make sure the schema name is in a list of valid schema names.  If the schema supplied does not match any in the list, it aborts the operation.  If it does match, it allows the function to continue.

HTH,

^ _ ^

Votes

Translate

Translate

Report

Report
Community guidelines
Be kind and respectful, give credit to the original source of content, and search for duplicates before posting. Learn more
community guidelines
Resources
Documentation