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.
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.
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,
^ _ ^
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
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,
^ _ ^
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!!!
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
Copy link to clipboard
Copied
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,
^ _ ^