AbstractCacheProvider.getOrSet(): local var unscoped when checking if null

Description

The issue here is that, if Lucee/Coldfusion scope cascading is on, then when 'target' is NULL, the engine will look up the scopes for it. If you have, for instance, FORM.target, due to submitting a link building form for example (https://presidecms.atlassian.net/browse/PRESIDECMS-1634), you'll get an unexpected result out of the cache.

Should use:

 

Activity

Show:
Luis Majano
July 31, 2019, 3:34 PM

Wow, this is so sucky, because, I thought the local scope was the first scope looked at. Is this not the case?

Brad Wood
July 31, 2019, 6:53 PM

It’s not a matter of the first scope to be looked in, it’s a matter of the other scopes that are searched. The code is simply ambiguous and needs to be made explicit. When local.target doesn’t exist, CF doesn’t stop, it keeps looking until it finds a variable such as form.target. Which leads to incorrect results when an unrelated variable of that name exists in another scope that is searched as Dom pointed out.

 

The fix is simply to add local. as Dom has suggested.

Brad Wood
July 31, 2019, 6:56 PM

I’d also like to point out, this closes some loopholes in which a malicious user of a ColdBox site may be able to “inject” variables into an app by adding form or url vars that will get accidentally picked up by the framework. I haven’t tried it yet to see if it’s actually exploitable, but the possibility is there.

Luis Majano
August 3, 2019, 2:16 PM

But the tremendous issue here is that we will have to scope EVERY stupid local variable, if not, we have the same issue everywhere.

Brad Wood
August 3, 2019, 6:43 PM

No, I don’t think you understand. This does not apply to every variable ever, just variables which may be null. Dom already identified all of these and addressed this in his pull. We simply need to merge it now.

Assignee

Luis Majano

Reporter

Dominic Watson

Labels

None

Fix versions

Priority

Major
Configure