mozilla
Your Search Results

    Appendix C: Avoiding using eval in Add-ons

    Using eval in add-ons is almost always unnecessary, and many times even a security vulnerability. Moreover, code using eval is harder to parse for a human mind, is often pretty complex, and relies on assumptions that are not necessarily true in the future or even now.

    This article is aimed at presenting alternatives to common eval uses in add-ons and other Javascript code.

    Add-on authors are strongly encouraged to update their code to eliminate all instances of eval, no matter if the add-on is to be hosted in the Mozilla Add-ons Gallery or not. In order to host your add-on with Mozilla it is crucial to minimize or completely eliminate eval use in order to receive a positive review and have your add-on made public. If it cannot be easily proven that an eval() call is benign and necessary because there are no viable alternatives, a Mozilla Add-ons Gallery reviewer will generally reject the submission!

    Parsing JSON

    Mozilla provides native JSON support since Firefox 3. There is simply no need to parse JSON using eval. Additionally, parsing JSON retrieved from a remote location becomes a security vulnerability when parsed with the eval function. Basically, you are executing remote code with full chrome access; that is, introducing a remote code execution vulnerability. Even if you trust the remote server; for example, because it is one you rent and administer yourself, there is a huge security risk, because of, but not limited to:

    • You might discontinue your project or sell it, so that it is possible another person with malicious intentions takes over your domain.
    • The server and/or domain name might be compromised.
    • If using an unencrypted, insecure connection, a Man-in-the-middle attacker might replace the JSON with attack code before it arrives at the user.

    Use Native JSON

    Since Firefox 3.5 you should use JSON. Really, just use it!

    Note: Do not use JSON parsers implemented in JavaScript. These implementations are less efficient and often also contain serious security vulnerabilities. Such implementations are meant to be used within a very different security context, namely a website, where the origin of the data is usually known in all instances and where vulnerabilities would have a much smaller impact.

    What about JSONP?

    JSONP, which really is just another script tag containing generated, remotely retrieved code, is generally not secure. It might be possible to execute any retrieved JSONP code in an unprivileged Sandbox (which is complicated!), but if you have a choice, just avoid JSONP altogether!

    Passing around functions/code as strings

    Often you'll want to pass functions or code to other functions, most notoriously setTimeout and addEventListener. Often this is done passing around strings.

    // DO NOT USE!
    setTimeout("doSomething();", 100);
    addEventListener("load", "myAddon.init(); myAddon.onLoad();", true);
    setInterval(am_I_a_string_or_function_reference_qmark, 100); 
    

    That in itself is certainly not elegant, but it may also become a security issue if you pass code that was externally retrieved (or at least contains bits of externally retrieved data):

    // DO NOT USE!
    setTimeout("alert('" + xhr.responseText + "');", 100);
    // Attacker manipulated responseText to contain "attack!'); format_computer(); alert('done"
    setTimeout("alert('attack!'); format_computer(); alert('done');", 100);
    

    As a general rule of thumb, just don't pass code around as strings and execute it by calling eval, setTimeout and friends.

    Alternative: Use (anonymous) functions

    You can always create a small anonymous function to pass around instead. Closures will ensure the code is still valid, even if your outer function already returned from execution.

    addEventListener("load", function() { myAddon.init(); myAddon.onLoad(); }, true);
    function doXHR() {
      //...
      var response = xhr.responseText;
      setTimeout(function() { alert(response); }, 100);
    } 
    

    Alternative: Use Function.bind()

    Introduced in JavaScript 1.8.5

    Function.bind is a new utility function that you may use to (partially) bind parameters to functions.

    addEventListener("load", myAddon.init.bind(myAddon), true);
    setTimeout(alert.bind(null, xhr.responseText), 100);
    

    Function.bind() returns a new function. This is important to know in particular when you later want to call removeEventListener: You'll have to pass the resulting function from the first .bind() call. Calling .bind() again with removeEventListener will not work. Also, calling .bind() in a tight (inner) loop should be avoided for performance reasons, as calling .bind() does require some work and memory.

    Overriding/Extending/Amending existing functions

    A common thing add-ons do during their initialization is overriding/extending existing browser functions by using Function.toString/Function.toSource and eval to "string-patch" the function body.

    // DO NOT USE!
    var functionBody = gBrowser.addTab.toSource();
    var afterBracket = functionBody.indexOf("{") + 1;
    functionBody = functionBody.substring(0, afterBracket) + "myAddon.onAddTab(aURI);" + functionBody.substring(afterBracket);
    eval("gBrowser.addTab = " + functionBody);
    

    Of course, this not only looks messy, but can be quite error prone.

    • Other extensions might do something similar, trying to hook the same functions, but a little different, ending up with completely broken code.
    • The code is hard to read and by that hard to maintain and review. (The example is a quite simple one. In real life such code is often far more complex)
    • The code might break in the future, as certain assumptions might not longer be true, for example the function signature may change (aURI from above becomes aURL) or the function is replaced by a short-hand/arrow function:
      function addTab(aURI) tabBrowser.addTab(aURI);
      var addTab = (aURI) => tabBrowser.addTab(aURI);

    Same as with "Passing functions/code as strings" above, patching a function to with fragments of externally retrieved data will create security vulnerabilities.

    Alternative: Replace + Function.apply()

    You may replace the original function with a new function, keeping a reference to the original function which you then call from the new one.

    // Wrap in an anonymous function (which is called immediately).
    // This will encapsulate local variables such as _original and
    // not pollute the global namespace.
    (function() {
    
      // Keep a reference to the original function.
      var _original = gBrowser.addTab;
      
      // Override a function.
      gBrowser.addTab = function() {
        // Execute some action before the original function call.
        try {
            myAddon.onAddTab(arguments[0]);
        } catch (ex) { /* might handle this */ }
    
        // Execute original function.
        var rv = _original.apply(gBrowser, arguments);
    
        // Execute some action afterwards.
        try {
          myAddon.doneAddTab(rv);
        } catch (ex) { /* might handle this */ }
    
        // return the original result
        return rv;
      };
    
    })();
    

    This is admittedly a bit more verbose, but at the same time it should be easier to understand.

    • You don't have to parse what the resulting function will look like in your mind.
    • There won't be any problems if various Add-ons override the same function (using this method).
    • You don't have to care about parameter naming (or changes in naming in future application versions) or short-hand/arrow functions.
    Note: It is not safe to remove such an override again, as this method constitutes in a single-linked function chain. If you want to disable your overrides again, then use a flag indicating that, or similar. At the same time, it is also not safe to "un-string-patch" a function, for the exact same reason.
    Note: There are still some scenarios where incompatibilities may arise, such as trying to cancel the function call under a certain set of conditions when other Add-ons have overridden the same function. Again, this is a problem with the "string-patch" method, too. How to handle this is inter-Add-on specific and not addressed in this article.

    Overriding/Extending/Amending existing objects (or object properties)

    Again, you might be tempted to uneval(), string replace and eval() existing objects to override them (or at least some properties).  Of course, the issues given in the previous section about functions remain.

    Alternative: Just assign

    Most of the time, assigning properties will just work.

    someExistingObject.someProperty = "abc";
    // We already demonstrated with with functions in a previous example
    

    Alternative: Object.defineProperty()

    For most objects it is possible to (re-)define properties with the Object.defineProperty() API, which allows to not override values, but also lets you define getters and setters.

    Alternative: Proxy

    If you cannot override objects or properties easily, e.g. because the object is frozen or sealed, then creating a Proxy around the object might be a viable approach.

    Triggering event handlers

    Sometimes scripts might want to manually trigger an event handler that is defined directly in the XUL document. Consider the following XUL fragment throughout the rest of this section.

    <menuitem id="mymenu" oncommand="executeSomething(); executeSomethingElse();"/>
    <label id="mylabel" onclick="executeSomething(); executeSomethingElse();"/>
    

    Add-on authors commonly use eval to trigger the handlers.

    // DO NOT USE
    eval(document.getElementById("mymenu").getAttribute("oncommand"));
    eval(document.getElementById("mylabel").getAttribute("onclick"));
    

    Alternative: Dispatch real events

    Dispatching real events has the added bonus that all other event listeners for that Element (and the corresponding bubbling/capturing chain) will fire as well, so this method will have the closed resemblance to a real user event.

    // Fake a command event
    var event = document.createEvent("Events");
    event.initEvent("command", true, true);
    document.getElementById("mymenu").dispatchEvent(event);
    
    // Fake a mouse click
    var mouseEvent = document.createEvent("MouseEvents");
    event.initMouseEvent("click", true, true, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
    document.getElementById("mylabel").dispatchEvent(mouseEvent);
    

    Please see the corresponding documentation on how to use and initialize particular event types.

    Alternative: Element.click()

    XUL and HTML elements have a pre-defined .click() method that will fire a synthesized click event.

    document.getElementById("mybutton").click();
    

    Alternative: Element.doCommand()

    XUL elements that have a command (oncommand or command attribute) assigned will also have a working doCommand method.

    document.getElementById("mymenu").doCommand();
    

    Alternative: Just hard-code what should happen

    If you know that the current version of the application will call { executeSomething(); executeSomethingElse();} then just do the same in your code. Of course, you'll need to update your add-on if the application changes.

    Accessing properties via computed names

    Not that common anymore, but still existing, are Add-Ons or other Javascript programs that access object properties using eval when the property name is not a literal, but computed on the fly.

    //DO NOT USE!
    eval("myAddon.phrases.word" + word + " = '" + phrase + "';");

    Again, this is not only unnecessarily hard to parse for a human, but may also contain security vulnerabilities if you compute the names using external data.

    Alternative: Using bracket-access to object properties

    Object properties can always accessed using the bracket syntax:

    obj["property"] === obj.property
    

    Hence the following will work just fine without having to resort to eval.

    myAddon.phrases["word" + word] = "phrase";
    

    If there is no way around eval()...

    Please consider creating an unprivileged Sandbox and execute your code by calling evalInSandbox.

    DId I mention that you should use Native JSON methods to parse JSON data?

    Special thanks

    Special thanks goes to Wladimir Palant of Adblock Plus, who wrote an article years back which heavily inspired this one.

    See also

    Document Tags and Contributors

    Last updated by: fscholz,
    Hide Sidebar