Potentially vulnerability using setInterval in Firefox addon?

Question

I've written a Firefox addon for the first time and it was reviewed and accepted a few month ago. This add-on calls frequently a third-party API. Meanwhile it was reviewed again and now the way it calls setInterval is criticized:

setInterval called in potentially dangerous manner. In order to prevent vulnerabilities, the setTimeout and setInterval functions should be called only with function expressions as their first argument. Variables referencing function names are acceptable but deprecated as they are not amenable to static source validation.

Here's some background about the »architecture« of my addon. It uses a global Object which is not much more than a namespace:

if ( 'undefined' == typeof myPlugin ) {
    var myPlugin = {
        //settings
        settings : {},

        intervalID : null,

        //called once on window.addEventlistener( 'load' )
        init : function() {
            //load settings
            //load remote data from cache (file)

        },

        //get the data from the API
        getRemoteData : function() {
            // XMLHttpRequest to the API
            // retreve data (application/json)
            // write it to a cache file
        }
    }

    //start 
    window.addEventListener(
    'load',
    function load( event ) {
        window.removeEventListener( 'load', load, false ); needed
        myPlugin.init();
    },
    false
);
}

So this may be not the best practice, but I keep on learning. The interval itself is called inside the init() method like so:

myPlugin.intervalID = window.setInterval(
    myPlugin.getRemoteData,
    myPlugin.settings.updateMinInterval * 1000 //milliseconds!
);

There's another point setting the interval: an observer to the settings (preferences) clears the current interval and set it exactly the same way like mentioned above when a change to the updateMinInterval setting occures.

As I get this right, a solution using »function expressions« should look like:

myPlugin.intervalID = window.setInterval(
    function() {
        myPlugin.getRemoteData();
    },
    myPlugin.settings.updateMinInterval * 1000 //milliseconds!
);

Am I right?

What is a possible scenario of »attacking« this code, I've overlooked so far?

Should setInterval and setTimeout basically used in another way in Firefox addons then in »normal« frontend javascripts? Because the documentation of setInterval exactly shows the way using declared functions in some examples.

score 2 · Accepted Answer · answered Apr 08 '13 at 20:40

Am I right?

Yes, although I imagine by now you've tried it and found it works.

As for why you are asked to change the code, it's because of the part of the warning message saying "Variables referencing function names are acceptable but deprecated as they are not amenable to static source validation".

This means that unless you follow the recommended pattern for the first parameter it is impossible to automatically calculate the outcome of executing the setInterval call.

Since setInterval is susceptible to the same kind of security risks as eval() it is important to check that the call is safe, even more so in privileged code such as an add-on so this warning serves as a red flag to the add-on reviewer to ensure that they carefully evaluate the safety of this line of code.

Your initial code should be accepted and cause no security issues but the add-on reviewer will appreciate having one less red flag to consider.

Given that the ability to automatically determine the outcome of executing JavaScript is useful for performance optimisation as well as automatic security checks I would wager that a function expression is also going to execute more quickly.

Potentially vulnerability using setInterval in Firefox addon?

1 Answers1