Apéndice C: Evitar usar eval en los Add-ons

Imagen:traduccion-pendiente.png Esta página está traduciéndose a partir del artículo Appendix_C%3A_Avoid_using_eval_in_Add-ons, razón por la cual puede haber algunos errores sintácticos o partes sin traducir. Puedes colaborar continuando con la traducción

Using eval in extensions 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 use 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.

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 native JSON. In Firefox 3.0, you may take a look at nsIJSON instead. Using native JSON has the added benefit that it is better when validating the input and also a lot faster.

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.

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 achieved by a hack "function/code as string" technique.

// 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 construct the passed code with externally provided 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);

Overriding/Extending 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, 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 shorthand not containing bracket:
    function addTab(aURI) tabBrowser.addTab(aURI);
      

Like with "Passing functions/code as strings" above, patching function to include some external 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.

(function() {
  var _original = gBrowser.addTab; // Reference to the original function
  gBrowser.addTab = function() {
    // Execute before
    try {
        myAddon.onAddTab(arguments[0]);
    } catch (ex) { /* might handle this */ }
    // Execute original function
    var rv = _original.apply(gBrowser, arguments);
    // execute afterwards
    try {
      myAddon.doneAddTab(rv);
    } catch (ex) { /* might handle this */ }

    // return the original result
    return rv;
  };
})();

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

  • You don't have to parse in your mind what the resulting function will look like.
  • There won't be any problems if various Add-ons employ this method with the same function.
  • You don't have to care about parameter naming or short-hand 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.

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

Elements that have a command (oncommand) assigned will also have a working doCommand method.

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

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 just work without having to resort to eval.

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

Special thanks

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

See also

Etiquetas y colaboradores del documento

Colaboradores de esta página: camilourd, Wladimir_Palant
Última actualización por: Wladimir_Palant,