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.

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!

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()

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);

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.

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

• Native JSON
• nsIJSON
• JSON
• Function.bind
• Function.apply
• Element.doCommand
• document.createEvent
• event.initEvent