Is this a bad use of Javascript's eval()?

Question

I've read a few things about Javascript's eval, so I think my use makes sense.

I'm in an information security class, and we're doing some statistical analysis on lottery cards. However, ours are bingo cards (and probably completely unlikely to be prey to this particular situation). However, our current process is severely buggy, so I had the idea of storing the card data on my server, and allow classmates to write javascript functions in a textarea and then push a button that would just eval(textarea.value). That would allow us to do some fairly useful computations on the data without too much problem. I also had the idea of "storing procedures" as text that they could load into the textarea and then evaluate.

Is this a bad area for eval()? Or a somewhat grey area? Also, what are some of the potential problems I could face? Obviously they could potentially bork their view of the page, but as long as I'm not doing anything stupid, a simple page reload should fix that, correct?

Thanks!

Answer 1

If the user input being eval()'d is always coming from the user, that should be fine, the only malicious JavaScript they can then run is on themselves (which they could do anyway).

If it speaks to server side code, make sure it is validated like any other external input.

You need to also ensure it can not be pre-filled from anything like a GET param etc.

Update

If you are only running basic math with variables, you could use a regex to ensure the value is clean from non arithmetic and then eval() it.

For example, your equations can have placeholders CARD1 and CARD2.

var textarea = document.getElementsByTagName('textarea')[0],
    placeholders = ['CARD1', 'CARD2'];

document.getElementsByTagName('button')[0].onclick = function() {

    var value = textarea.value;

    // Strip placeholders
    var placeholderStripped = value.replace(new RegExp(placeholders.join('|'), 'g'), '');

    var safe = placeholderStripped.match(/^[\d\+\-\/\*\(\)]+$/);

    if (safe == null) {

        alert('Not a valid equation');
        return;

    }

    for (var i = 0, placeholdersLength = placeholders.length; i < placeholdersLength; i++) {
        value = value.replace(new RegExp(placeholders[i], 'g'), document.getElementById(placeholders[i].toLowerCase()).value);
    }

    document.getElementById('result').innerHTML = eval(value);

}

jsFiddle.

• Use simple placeholder names, and all caps for convenience. If you use more complicated names, you may need to use a regex escaping function before you join() them.
• We allow numbers 0-9, +, -, /, *, ( and ). We don't allow for decimals currently. That can be added if needed.
• When we finally eval() the input, it should only contain placeholder names (which will be substituted) and the basic arithmetic characters above.
• The code posted above is highly dependent on the jsFiddle context. Of course, you will never allow placeholder names to be arbitrary - make them what they should be.

Answer 2

I don't want to be melodramatic but this is probably one of the worst usages of eval() since you will be executing arbitrary, user-supplied JavaScript. This would allow a user to execute code on your domain which would make an XSS attack rediculously simple.

Edit: On second thought maybe I am being too dramatic. As long as you aren't persisting these functions and they are simply being eval'd on the client's machine then you aren't really allowing anything that the user can't already do in their local JavaScript console in the browser. Just be sure not to save these functions for other users to use later.