Is this use of Javascript eval() 100% safe?

Question

I'm writing a PHP library which generates Javascript code.

The Javascript code has a number of components named component001, component002, etc.

Pages are loaded dynamically via AJAX.

I need to pass the name of the component via URL variable which is then evaled() by the script.

The only way I am protecting what is being evaled is with the regular expression ^component[0-9]{3}$: if it passes it gets evaled, otherwise it does not.

To me this is 100% safe since nothing will get executed unless it is simply the name of one of my known components, or is there something about the eval() command that could be exploited in this code sample, e.g. regex injection, some kind of cross site scripting etc.?

window.onload = function() {

    // *** DEFINED IN ANOTHER JAVASCRIPT FILE:
    var component001 = 'testing111';
    var component002 = 'testing222';
    var component003 = 'testing333';

    var APP = {};

    APP.getUrlVars = function() {
        var vars = [], hash;
        var hashes = window.location.href.slice(window.location.href.indexOf('?') + 1).split('&');
        for(var i = 0; i < hashes.length; i++) {
            hash = hashes[i].split('=');
            vars.push(hash[0]);
            vars[hash[0]] = hash[1];
        }
        return vars;
    }

    APP.getUrlVar = function(name, defaultValue) {
        defaultValue = (typeof defaultValue == 'undefined') ? '' : defaultValue;
        var vars = APP.getUrlVars();
        if(vars[name] === undefined)
        {
            return defaultValue;
        } else {
            return vars[name];
        }
    }

    APP.safeEval = function(nameOfComponent) {
        var REGEX_VALID_NAME = /^component[0-9]{3}$/;
        if(REGEX_VALID_NAME.test(nameOfComponent)) {
            return eval(nameOfComponent);
        } else {
            return 'ERROR';
        }

    }

    // *** JAVASCRIPT FILE LOADED VIA AJAX:

    var nameOfComponentToDisplay = APP.getUrlVar('compname', 'component001');
    var component = APP.safeEval(nameOfComponentToDisplay);
    document.write(component);

}

Instead of using eval for this, I'd use the square bracket notation to call your function (e.g. APP[nameOfComponent]();) — Marcel Korpel
– Marcel Korpel, Commented Dec 21, 2010 at 14:42
regex and eval() are both open ended tools that introduce risk. The problem you describe need not embrace that risk. Instead - as @Marcel Korpel and @ChaosPandio have writ - use string matching and function invoking to avoid. — orangepips
– orangepips, Commented Dec 21, 2010 at 14:48
The only excuse I can think of to use eval() is if you're writing something like the Firebug console. As other answers show, most places where you might consider using eval can be acheived better without it anyway. And in ihe few remaining scenarios where eval() may genuinely be useful, it should certainly not be considered safe. — Spudley
– Spudley, Commented Dec 21, 2010 at 14:53

ChaosPandion · Accepted Answer · 2010-12-21 14:42:33Z

15

There is almost zero reasons to use eval and I think that this is not one of them. Remember that all objects act like dictionaries so you can simply do something like this:

var components = {
    component001 : 'testing111',
    component002 : 'testing222',
    component003 : 'testing333'
};

APP.safeEval = function(nameOfComponent) {
    var result = components[nameOfComponent];
    if(result) {
        return result;
    } else {
        return 'ERROR';
    }
}

answered Dec 21, 2010 at 14:42

ChaosPandion

78.4k18 gold badges122 silver badges159 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Georg Schölly Over a year ago

... and you should probably be using an array anyway if you've got a list of objects.

Pointy · Accepted Answer · 2010-12-21 14:44:43Z

4

Well, if all there is is a name, then

  eval(component101)

won't do anything anyway, so it seems safe. Maybe you meant

  return eval(nameOfComponent + '()');

If so, then I don't see why you don't just put your components in a namespace object. Then you wouldn't need eval at all:

  return components[nameOfComponent]();

If they're not functions, then the same thing applies, but you'd leave off the "()".

answered Dec 21, 2010 at 14:44

Pointy

415k62 gold badges600 silver badges631 bronze badges

Comments

Emmett · Accepted Answer · 2010-12-21 14:54:22Z

3

If the variables are defined in another javascript file and contain only numbers and letters, then they are part of the global namespace. As such, they can be accessed as properties of the window object (no need for eval!):

if (typeof window[nameOfComponent] !== 'undefined')
    return window[nameOfComponent]
return 'ERROR';

edited Dec 21, 2010 at 14:54

answered Dec 21, 2010 at 14:47

Emmett

14.4k12 gold badges58 silver badges82 bronze badges

2 Comments

ChaosPandion Over a year ago

If you are using anything less than ECMAScript 5 you want to use typeof x === "undefined" because undefined is a mutable variable.

Mike Samuel Over a year ago

If you want to know whether there is a property with that name, then nameOfComponent in window will do that and not fail around defined properties whose value is undefined.

Collectives™ on Stack Overflow

Is this use of Javascript eval() 100% safe?

3 Answers 3

1 Comment

Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related