CFQUERYPARAM_REQ false positives
TheRealAgentK opened this issue · 10 comments
I found a bunch of false positives when running CFQUERYPARAM_REQ, all on MS SQL Server. I know on MySQL apparently one can cfqueryparam in the SELECT statement for instance, but MS SQL Server doesn't allow that.
Is there any chance a change/improvement to the SQL parsing process could weed out some or all of those kinds of reportings?
-
SELECT TOP #arguments.numberOfRecords# ...
-
SELECT something FROM #application.config.LinkedServerName#.somethingelse.dbo.Comment C WITH (NOLOCK)...
-
<cfqueryparam value="Data copied from #variables.siteDetailList[arguments.siteID]["name"]# - #dateFormat(now(),"DD/MM/YYYY")#" cfsqltype="varchar">
-
OPEN SYMMETRIC KEY #config.symmetrickey#
DECRYPTION BY CERTIFICATE #config.dbCertificate#
...
CLOSE SYMMETRIC KEY #config.symmetricKey# -
SELECT '#arguments.additionalValue#' AS aID, '#arguments.additionalOption#' AS trans ...
#3 should not flag since it is in a cfqueryparam tag.
But all of the others are potential SQL injection vectors, unless I am missing something.
I would be comfortable excluding variables from specific scopes (like application), but #arguments.someValue# is exactly what this rule is intended to flag.
Ha, yeah - those are interesting ones.
I agree - number 3 is an obvious false positive.
The issue with any of the other ones is that it seems cfqueryparam can't be used to fix them, at least not on SQL Server, because it doesn't seem to accept query params in the Select part of a query. Some people on the web claim that MySQL does allow that, for instance.
For 1, I could at least improve the situation by using val() around it - CFLint would probably still report it though. 2,4 and 5, I'm not sure how to deal with them.
So - you're kind of right, maybe the term "false positives" is wrong for those - but if they're not cases for cfqueryparam to fix, I'm not sure if the cfqueryparam rule should pick them up.
I actually think that excluding certain scopes is not a good idea, probably not even using the application or server scopes etc.
This would be less of an issue if we could get the ignoring to work, I reopened #195 for exactly this case with some test cases in the dev branches.
Yup, can confirm that sub-issue 3 from above is fixed and that ignoring on a query-level now is working. Opened #295 for a more fine-grain ignore handling within SQL - but I'm not sure if that's feasible.
@TheRealAgentK What version of SQL Server are you using? I've tested on SQL Server 2012 with CF9 and this is what I found.
1. works with parentheses.
SELECT TOP (<cfqueryparam cfsqltype="CF_SQL_INTEGER" value="#arguments.numberOfRecords#">) *
5. works as is.
SELECT <cfqueryparam cfsqltype="CF_SQL_VARCHAR" value="#arguments.additionalValue#"> AS aID, <cfqueryparam cfsqltype="CF_SQL_INTEGER" value="#arguments.additionalOption#"> AS trans
Re 1: Ah, we didn't try with parentheses around the number, will give that a try.
Re 5: Hmmm, I'm pretty sure we tried that and it didn't work. SQL Server 2012/2014 with CF11. Need to try that one again, too.
@TheRealAgentK I've found that not only does 5 work like that, but also in functions. For example, we have some queries that use things like
SELECT ISNULL(someValue, <cfqueryparam cfsqltype="CF_SQL_VARCHAR" value="#someDefaultValue#">) AS someValue
@KamasamaK 1 does indeed work fine, but I can't get 5 to work. Well, it does work syntactically, how would I get it to actually to get to not return the strings refunddate or member_length?
<cfset column = "refunddate">
<cfset column2 = "member_length">
<cfquery name="paymenthistory" datasource="something">
SELECT TOP (<cfqueryparam value="#numberOfRecs#" cfsqltype="cf_sql_integer">)
CASE WHEN (some condition) THEN something ELSE somethingelse END AS paiddate,
<cfqueryparam value="#column#" cfsqltype="cf_sql_varchar"> as refund,
<cfqueryparam value="#column2#" cfsqltype="cf_sql_varchar"> as length, ...
Running this query with cfqp essentially spits out the literal strings refunddate and member_length in each row of the query in those columns. What I want though is to dynamically pass in the column names to be used - and I still think that is not possible.
@TheRealAgentK Oh, I misunderstood the intent. Your example has them in single quotes, so it was misleading. cfqueryparam will only work for passing data, not database objects/object names. I'm no SQL Server expert, but I doubt you can do that without resorting to dynamic SQL. I don't even think there's a way to do this with stored procedure params and invoking directly in SQL Server (ignoring ColdFusion completely).
Yeah, I know. You can do it without cfqueryparam by just putting stuff in # pounds at that's what they've been doing quite extensively.