tracker issue : CF-4204121

select a category, or use search below
(searches all categories and all time range)
Title:

Locally scoped variables with null values may see scope bleed

| View in Tracker

Status/Resolution/Reason: Closed/Withdrawn/AsDesigned

Reporter/Name(from Bugbase): Dan S. / ()

Created: 03/26/2019

Components: Language, Scopes

Versions: 2016,2018

Failure Type: Data Corruption

Found In Build/Fixed In Build: CF10 - 2018 / CF 2018 Final Build

Priority/Frequency: Normal / All users will encounter

Locale/System: ALL / CentOS 7.3

Vote Count: 6

Problem Description:
When a locally scoped variable is set to a null value, it's value may end up non-null if a similarly named variable exists in the variables scope (or if searchImplicitScopes is true, any scope).

This is critically broken in all versions of ColdFusion from 10 to 2018, but ColdFusion 2016 behaves differently than all other the other implementations.

In ColdFusion 2016, any attempt to check if a variable's value is null in your function will cause CF to return the value for any matching named var in the "variables" scope. For example:

// this variable should be in the "variables" scoped
x = "variable not scoped correctly!";

function getNull(){
    return;
}

function test(){
    // since we're varscoping the variable, references to "x" in this function should reference local.x, not variables.x
    var x = getNull();

    if( isNull(x) ){
        return;
    } else {
        return x;
    }
}

If you run this function CF2016 will return "variable not scoped correctly!" instead of returning a null value. In all other versions of CF, the results will be null. 

If instead of using the value "x" in your local function, you use a unique value, the function will execute as expected.

However, if you attempt to reference the variable *before* checking if it's null, it will cause the variable to be reassigned. This is actually broken in CF10 - CF2018. For example:

// this variable should be in the "variables" scoped
x = "variable not scoped correctly!";

function getNull(){
    return;
}

function test(){
    // since we're varscoping the variable, references to "x" in this function should reference local.x, not variables.x
    var x = getNull();
    // by simply referencing the "x" variable before we check for null, when "x" is null, it forces "x" to go up the scope chain 
    var xxx = x;

    if( isNull(local.x) ){
        return;
    } else {
        return local.x;
    }
}

The line "var xxx = x" causes the value of "x" to be reassigned to "variable not scoped correctly!", even though it should be null. This causes the test() function to return "variable not scoped correctly!" instead of null.

This becomes very problematic in components that have accessors, for example:

component output="false" accessors="true" persistent="false" {
	property name="name" type="string" getter="true" setter="true";

	function getNull(){
		return;
	}

	function test(){
		var name = getNull(); // some method that might return null

		if( isNull(name) ){
			return;
		}

		return name;
	}
}

In this case, if the would call obj.setName("Hello World!"), then the test() method will return "Hello World!" instead of null. However if the result of getNull() is null, then test() should always return null.

In CF2016+, you can set the Application's searchImplicitScopes to "false" you can limit the affects of the bug to only variables within the "variables" scope, but if you have not explicitly set searchImplicitScopes to "false", then anyone passing in a URL/FORM variable can trigger bugs in your code. 

For example, any function that contains a variable that *could* be null all you need to do is pass in a URL/FORM parameter with the same name as the function's variable and the value would become that you passed in and not the expected null.

The problem is this can cause very subtle bugs in your code, that are not easily traceable. I suspect many people have been affected by this bug.

You can run the examples below to see the bug in action.

Steps to Reproduce:

https://cffiddle.org/app/file?filepath=c8dfdbc4-2f3d-4e75-89ff-23fbea37d3f5/1816f8e4-0c07-41c5-ad0f-c37070cdac2e/01eae701-46ab-4075-9450-9e69c864b049.cfm

<cfscript>
    // this variable should be in the "variables" scoped
    x = "variable not scoped correctly!";

    function getNull(){
        return;
    }
    
    function testNullWithLocallyScopedVariableMatchesGlobalVariableName(){
        // since we're varscoping the variable, references to "x" in this function should reference local.x, not variables.x
        var x = getNull();

        if( isNull(x) ){
            return;
        } else {
            return x;
        }
    }
    
    function testNullWithUniquelyNamedLocallyScopedVariable(){
        // since there is no variable called "x1" in our variables scope, this works correctly
        var x1 = getNull();
        
        if( isNull(x1) ){
            return;
        } else {
            return x1;
        }
    }
    
    function testNullWithLocallyScopedVariableBrokenInAllVersionOfColdFusion(){
        // since we're varscoping the variable, references to "x" in this function should reference local.x, not variables.x
        var x = getNull();
        // by simply referencing the "x" variable before we check for null, when "x" is null, it forces "x" to go up the scope chain
        var xxx = x;

        if( isNull(x) ){
            return;
        } else {
            return x;
        }
    }
    
    function testNullEvenUsingLocalScopeDoesNotWork(){
        // since we're varscoping the variable, references to "x" in this function should reference local.x, not variables.x
        var x = getNull();
        // by simply referencing the "x" variable before we check for null, when "x" is null, it forces "x" to go up the scope chain
        var xxx = x;

        if( isNull(local.x) ){
            return;
        } else {
            return local.x;
        }
    }

    brokenOnlyInCF2016 = testNullWithLocallyScopedVariableMatchesGlobalVariableName();
    worksAsExpected = testNullWithUniquelyNamedLocallyScopedVariable();
    brokenInAllVersionsOfColdFusion = testNullWithLocallyScopedVariableBrokenInAllVersionOfColdFusion();
    evenUsingScopeDoesNotWork = testNullEvenUsingLocalScopeDoesNotWork();
</cfscript>

<cfoutput>
    <div>
        brokenOnlyInCF2016 = <cfif isNull(brokenOnlyInCF2016)>null<cfelse>#encodeForHtml(brokenOnlyInCF2016)#</cfif> (Should be null)
    </div>
    <div>
        worksAsExpected = <cfif isNull(worksAsExpected)>null<cfelse>#encodeForHtml(worksAsExpected)#</cfif> (works correctly)
    </div>
    <div>
        brokenInAllVersionsOfColdFusion = <cfif isNull(brokenInAllVersionsOfColdFusion)>null<cfelse>#encodeForHtml(brokenInAllVersionsOfColdFusion)#</cfif> (Should be null)
    </div>
    <div>
        evenUsingScopeDoesNotWork = <cfif isNull(evenUsingScopeDoesNotWork)>null<cfelse>#encodeForHtml(evenUsingScopeDoesNotWork)#</cfif> (Should be null)
    </div>
</cfoutput>

Actual Result:

brokenOnlyInCF2016 = variable not scoped correctly! (Should be null)
worksAsExpected = null (works correctly)
brokenInAllVersionsOfColdFusion = variable not scoped correctly! (Should be null)
evenUsingScopeDoesNotWork = variable not scoped correctly! (Should be null)

Expected Result:

brokenOnlyInCF2016 = null (Should be null)
worksAsExpected = null (works correctly)
brokenInAllVersionsOfColdFusion = null (Should be null)
evenUsingScopeDoesNotWork = null (Should be null)

Any Workarounds:
The only thing that appears to avoid the issue is to always scope every single instance of a variable.

Attachments:

Comments:

The title of this bug is bad. I should have made it: Locally scoped variables with null values may see scope bleed
Comment by Dan S.
30570 | March 26, 2019 08:11:40 PM GMT
Just to make sure this is clear, if the Application.cfc does not explicitly declare the following: this.searchImplicitScopes = false; Then this bug can be exploited by passing in URL/FORM parameters. For example, let's say you had some method in a component that look like this: function parseEmail(required string address){ var InternetAddress = createObject("java", "javax.mail.internet.InternetAddress").init(javaCast("string", arguments.address)); var email = InternetAddress.getAddress(); return email; } The results of InternetAddress.getAddress() can be null if an address cannot be extracted. This means if someone added a URL parameter of "?email=hacked@own.ed" to the URL of the request, if InternetAddress.getAddress() was null, then the results of parseEmail() would not be null but would be "hacked@own.ed". This is a basic example, but the ramifications could be worse—especially if you were using some Java libraries for hashing passwords, that if feed certain input could return null values. In theory, one could bypass logging into the system by forcing input that would return null and then overriding the variable output.
Comment by Dan S.
30574 | March 27, 2019 11:49:16 AM GMT
In ColdFusion 2018, with null support ON I am seeing the expected output. Without null support the explanation is mentioned below- Explanation for the behavior: -----------------------------------------   # Prior to null support / when null support is OFF - if you search for a variable and lets say it is found in functional local scope with value null, search will consider it as not found because a variable with value null is undefined. Since it will not be found in function local scope, so it will move ahead and find that in variablesScope and it will find a non null value there and return the same. # With null support ON - A variable with value null is a valid value. So, it will be found in function local scope and will just return that. That's why we will see null as return.
Comment by Vijay M.
30772 | May 17, 2019 08:13:05 AM GMT
I see that this is fixed in CF2018. Will it also be fixed in CF2016?
Vote by James M.
30782 | May 20, 2019 09:43:46 PM GMT
When can we expect a fix for CF2016?
Comment by Dan S.
30793 | May 21, 2019 11:51:29 AM GMT
FYI: I received an email on Wed 7/3/2019 2:33 AM Pacific with the following info, but don't see it reflected anywhere on this webpage. Locales updated to an empty value Failure Type updated from 'Data Corruption' to an empty value
Comment by James M.
30986 | July 03, 2019 01:47:48 PM GMT
Is there an update on when this will be fixed for CF2016?
Comment by Dan S.
31157 | August 23, 2019 03:04:36 AM GMT
+1 - IsABug. CF2016 behaves differently than earlier and later versions of CF, but should behave the same. Needs re-opened/fixed for CF2016 specifically.
Vote by Aaron N.
31176 | August 24, 2019 07:37:51 AM GMT
Hi Vijay, Please unzip attached 20190824_4204121.zip and compare the result across CF11, CF2016 and CF2018. You'll see the CF11 and CF2018 result matches, but the CF2016 result differs and needs fixed. This ticket's current "Fixed In Build:CF 2018 Final Build" contradicts its Closed/Withdrawn/AsDesigned. +1 for this ticket's "Fixed In Build" to reflect which _CF2016_ update this is fixed in. And if not fixed in CF2016, this ticket would need re-opened/fixed for CF2016. Thanks!, -Aaron
Comment by Aaron N.
31175 | August 24, 2019 07:47:07 AM GMT