Javascript security question / Using eval()

Question

I'm seeing code in the following form - is such use of eval() safe?

function genericTakeAction(frm_name,id,pagenum,action)
{
    var rset=eval("document."+frm_name);

    var x=eval("document."+frm_name+".edit_key");
    var y=eval("document."+frm_name+".cAction")
    if(x)
        x.value=id;
    if(y)
        y.value=action;

    page_list(pagenum);
}

Its used as:

  <a href="javaScript:;" onClick="genericTakeAction('frmSearch',
  '<?php echo $rec_id;?>','<?php echo $pagenum?>','makeOpen')" 
  class='link6'>Make Open</a>

This is probably #1 on the list of unnecessary uses of eval. See Gareth's answer for why. — T.J. Crowder
– T.J. Crowder, Commented Aug 5, 2011 at 13:06

Gareth · Accepted Answer · 2011-08-05 13:16:11Z

5

Whether it's right or wrong, it's needlessly complicated.

function genericTakeAction(frm_name,id,pagenum,action)
{
    var rset = document[frm_name];

    var x = rset.edit_key;
    var y = rset.cAction;

    if(x)
        x.value=id;
    if(y)
        y.value=action;

    page_list(pagenum);
}

This works because in JavaScript, you can access an object's properties in one of two ways: Either using dotted syntax and a literal identifier, e.g. x = obj.foo;, or using bracket syntax and a string identifier, e.g. x = obj["foo"];. (Note how foo was not in quotes in the first one, but was in quotes for the second; but both do exactly the same thing. Also note that since the property name is a string in the second case, you can use any expression that results in a string, so y = "f"; x = obj[y + "oo"]; also works.)

P.S. It's wrong

edited Aug 5, 2011 at 13:16

answered Aug 5, 2011 at 13:05

Gareth

139k36 gold badges152 silver badges158 bronze badges

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

4 Comments

jAndy Over a year ago

However, it's necessary to check if document[frm_name] is actually an object / not undefined. Otherwise i'll throw a type error on accessing those propertys.

T.J. Crowder Over a year ago

@jAndy: True, but then, so would the eval version. :-)

jAndy Over a year ago

@TJCrowder: sure, but I guess everyone gets my point. Why leaving bugs within an "answer" ?

Gareth Over a year ago

I wanted to keep a line-for-line equivalence with the question, to highlight the relevant point (that the eval use is unnecessary).

Spudley · Accepted Answer · 2011-08-05 13:12:00Z

eval() is generally frowned upon because, as you are already aware, it is considered unsafe.

In the browser environment, however, it is less of an issue, because in fact, any user could eval() any code they wanted to, using tools like Firebug, etc.

There is still an issue, in that the eval() embedded in the code can be run without the user knowing that he was triggering an eval(), but it's still much less of an issue than in a server-side environment like PHP.

eval() is actually typically used as you've shown to run JSON code being returned from a server-side request. Newer browsers can import JSON more safely using a dedicated JSON parse() function, but older browsers do not have this function and are forced to use eval() for this. Most JSON libraries have eval() in their code somewhere for this reason, but will generally do some sanitisation of the input before running it through eval().

Pierre Ernst · Accepted Answer · 2011-08-05 18:31:41Z

1

Even if it might look a little bit convoluted, as others have already mentioned, from a pure security perspective, you have to make sure that the 'frm_name' parameter of the genericTakeAction() function can never contain user-supplied data.

In your example, the 'frm_name' parameter contains the hard-coded literal 'frmSearch'. So it is ok as long as this genericTakeAction() function does not get called somewhere else with user-supplied data for the 'frm_name' parameter.

See http://en.wikipedia.org/wiki/Cross-site_scripting#Traditional_versus_DOM-based_vulnerabilities

answered Aug 5, 2011 at 18:31

Pierre Ernst

5243 silver badges7 bronze badges

Collectives™ on Stack Overflow

Javascript security question / Using eval()

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related