Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scenario where cfqueryparam check isn't flagging variables #18

Open
jbartlett777 opened this issue Jan 21, 2025 · 1 comment
Open

Scenario where cfqueryparam check isn't flagging variables #18

jbartlett777 opened this issue Jan 21, 2025 · 1 comment

Comments

@jbartlett777
Copy link

jbartlett777 commented Jan 21, 2025

Fixinator 5.0.1 has found and fixed MANY issues in a legacy app but I had a script where it caught the last variable in the query but not the first two. I removed code until I was left with the following and still able to reproduce

<CFSET cSet="">
<CFSET cVal="">
<cfquery name="qryUpdate" datasource="#db_source#" >
	pr_CO_UpdateSettings '#cSet#', '#cVal#', '#session.ML_USER_ATTUID#'
</cfquery>

If I remove either or both of the CFSET commands, it catches the variables correctly.

While testing the above code and trying different variations, I noticed it wouldn't catch single-character variables

<cfquery name="qryUpdate" datasource="#db_source#">
	pr_CO_UpdateSettings '#a#', '#b#', '#c#', #d#, #e#
</cfquery>

The code above was scanned in an isolated directory in case that has any impact, such as if there's variable scanning logic elsewhere.

@pfreitag
Copy link
Member

If I scan with confidence=low it will find all of these. In your first example because you set cSet and cVal in the cfset to an explicit value it lowers the confidence level, because if you have code like this:

<cfset someValue = 3>
<cfquery>
SELECT * FROM table
WHERE someValue = #someValue#
</cfquery>

That is not technically vulnerable to SQL Injection, since someValue is set to a static value.

As for the single variable variables, those are also found on confidence=low but I think those should not require low confidence so I'll see if I can fix that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants