Five wrong reasons to use eval () in an extension

Document directory

• Wladimir palant

Http://adblockplus.org/blog/five-wrong-reasons-to-use-eval-in-an-extension

 

Wladimir palant

One of the most overused JavaScript features iseval()Function. I have seen it used in very extends extensions but only a few actually had a good reason to use it. So I want to go through allWrongReasons one wocould useeval().

1. Processing JSON data

The JSON format got popular for storing data lately. Its most convenient feature: it is so easy to parse! I mean, all you have to do is to writedata = eval(json)And you are done.

What's the catch? Right,jsonVariable might contain something like{foo: "bar" + alert(Components.classes)}And then you will end up running JavaScript code, something that you didn't intend. so this way of parsing JSON data is absolutely unsuitable for data coming from untrusted sources. and guess what: if you are a Firefox extension, data coming fromAnyWeb server is untrusted. even if it is "Your" Web server, it might be hacked or the data might have been manipulated on its way to the user (especially for unencrypted connections ), and you really don't want to put users at risk when something like that happens.

But there is more: Even the data that your extension wrote itself (e.g. extension state saved on browser shutdown) cannot be always considered trusted. often it will write out data that it has ed from the web in one way or other. if there is a bug in the way JSON is written and that data breaks out of JavaScript strings you will unintentionally run JavaScript code when you "parse" That JSON. this means that it is always better to use methods dedicated to processing JSON that will no longer run javascript when processing ing invalid data.

2. Using object properties when property name is determined dynamically

What if your code needs to accessobj.fooNWhere "N" is the value of the Variablen? Meaning that the name of the property you have to access is not known in advance but has to be determined dynamically. Extensions will sometimes do things likeeval("obj.foo" + n)To solve this problem. Here the extension wocould need to verify that valuenCannot contain anything malicious-but how?

Fortunately, that question doesn't need to be answered. There is a better way, one only has to remember that all objects in JavaScript are associative arrays. In other words,obj.fooAndobj["foo"]Are exactly the same thing, each property is at the same type an array member. So to solve the problem above you simply need to writeobj["foo" + n]And that operation will always access a property, never do anything else.

But what about methods? Methods In JavaScript are properties as well, only difference is that their value is a function. You can use the method function. Call () to call that function with the correct value ofthisPointer:

var method = obj["foo" + n];
method.call(obj, param1, param2);

Or in a more compressed way:

obj["foo" + n](param1, param2);

Same approach can even be applied to global variables and functions. Those are all properties of the "Global Object" which can usually be referenced by the namewindow. Sowindow.fooOrwindow["foo"]Will give you the value of the global variablefoo.

3. Telling functions what they shoshould do when they are done

One pattern that I wowould occasionally see is calling a function like this:

foo("window.close()");

On other occasions the same function wocould be called with different JavaScript code as parameter. And when the function is done it wocould calleval()On its parameter to run the specified action.

Obviusly, there are no security issues here, so what's wrong with that approach? Actually, several things:

• This code will not be compiledeval()Is called. that means while the javascript interpreter will report syntax errors for the rest of the code immediately when the script is loaded, syntax errors in the function parameter will be reported late and might go unnoticed simply because you never tested the code path that will run that code.
• Another consequence is that for errors in that Code the javascript interpreter cannot report the correct source file and line number, it simply doesn't know where that code came from. debugging such errors is no fun.
• Finally, passing parametersfoo()And the executed code is non-trivial and requires uugly workarounds.

Fortunately, all these problems go away if you use closures. Here is a rewritten an slightly extended version of the code above:

foo(function(error)
{
  alert(error);
  window.close();
});

And the Functionfoo()Wocould look like this:

function foo(callback)
{
  ...
  callback("Full success");
}

4. Triggering inline event handlers in HTML or XUL

Let's assume we have a button like this:

<button id="button" oncommand="doSomething();"/>

Then why not doeval(document.getElementById("button").getAttribute("oncommand"))To trigger that event handler? Typically, extensions will do this to trigger event handlers on elements that aren't their own. however, generating a "command" event is much easier and will work regardless of how the event handler is defined:

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

The methoddoCommand()Is available for all XUL elements. As to other events, it is better to generate a real event object using document. createevent ()-because the event handler might Except CT one. For example:

var event = document.createEvent("MouseEvents");
event.initMouseEvent("click", true, true, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
document.getElementById("button").dispatchEvent();

But what if you defined your own custom attribute "onfooaction" that isn't associated with any real event? Even in that situation usingeval()Isn't the best choice because then the code will execute in the context of the function callingeval(). So if the event handler is using the global variablefooBut your function calling that event handler has a local variablefoo-The event handler will inadvertently access the local variable. And of course you cannot pass parameters to the event handler then. A better solution wocould be creating a function for the event handler:

var handler = new Function("param1", "param2", document.getElementById("button").getAttribute("onfooaction"));
handler("foo", "bar");

In this scenario the event handler will get "foo" as parameterparam1And "BAR" as parameterparam2(This is how the usual inline Event Handlers geteventParameter ).

5. Rewriting browser's functions

Occasionally I see code that goes like this:

gBrowser.foo = eval(gBrowser.foo.toString().replace("foo", "bar"));

I recommend a public spanking for everybody who is rewriting browser's functions like this. it is only slightly better than the extensions that simply replace browser's functions by their own. the assumption in both cases is that the code of the function being rewritten never changes-but what if it does? In the best-case scenario the extension will simply stop working, not much damage done. but it coshould just as easily break the browser. or, if the browser function changed to fix a security issue, the extension cocould reintroduce that issue.

In other words-don't do this. in most cases the idea is not to change the way the browser function works but to insert additional code before/after it runs. fortunately, there is a less dangerous way to achieve just this, you simply wrap the original function in your own, using closures again:

var origFoo = gBrowser.foo;
gBrowser.foo = function(param1, param2)
{
  if (param1 == "top secret")
    doSomethingBeforeFoo();
  var result = origFoo.apply(this, arguments);
  if (result == null)
    doSomethingAfterFoo();
  return result;
}

Note how function. apply () is used to call the original function will all parameters you got. even if that function has es only two parameters now this might change in a future browser version. your extension might now know what to do with the new parameters but it shoshould still pass them to the original function to avoid breaking it.

What about the valid uses of eval ()?

I don't think there are all that has valid uses foreval()Function. Some extensions allow users to enter JavaScript code that will be evaluated. Hereeval()Is justified though it might still be better create a function using function () constructor and pass any variables that script might need explicitly as function parameters.

Another possible useeval()Is declaring constants conditionally:

if (typeof MY_CONSTANT == "undefined")
  eval("const MY_CONSTANT = 'foo'");

That way if another script decides to declare the same constant you won't get a syntax error. I still consider this approach a hack however: If you are afraid of clashing with unknown scripts running in same namespace you shoshould make sure that your constants (as well as global variables) have unique names that other scripts won't use. and as to your own scripts, making sure that the script containing constant declarations isn' t loaded more than once shouldn't be too hard.

Finally, there are those obfuscated or "packed" scripts that make heavy useeval()To generate their code at runtime. while I see the value of "compressing" scripts on the web, doing the same in extensions makes very little sense. extensions are downloaded only once so saving two seconds of download time won't help anybody. on the other hand, the "packed" script will cause delays every time it needs to be loaded which might be pretty often.