Eval is evil?

  • Hi,

    I developed a plugin. One of its options is to let the user insert a JS script via textarea in the plugin. I push the script to PHP array, than use wp_localize_script to turn into a JS Object. The script runs before Contact Form 7 email message is sent:

    function wpcf7_redirect_mailsent_handler() {
	document.addEventListener( 'wpcf7mailsent', function( event ) {
		form = wpcf7_redirect_forms [ event.detail.contactFormId ];
		
		// Script to run after sent.
		if ( form.after_sent_script ) {
			eval( form.after_sent_script );
		}
	}
}

    Now, I have two questions

    1. Is this the best method? Is this safe? eval() has a bad reputation. If not,
      I’ll be happy to hear your suggestions.
    2. I sanitize the textarea of the script with sanitize_textarea_field to keep the spaces\rows in the textarea, but I also see it in the JS object as \r\n.
      So for example, I can run a script like this: eval("alert('hello');\r\nalert('world')");. It runs OK for now, but is there a better way?

    Thank you,
    Yuval.

Viewing 4 replies - 1 through 4 (of 4 total)
  • Moderator bcworkz

    (@bcworkz)

    Unless this plugin is just for your own site, one problem you’ll have is many hosts disable eval() on their servers. That pretty much overrides any other arguments pro or con.

    Why not just output the localized JS script as a string within script tags?

    add_action('wp_print_scripts', 'bcw_inject_js');
function bcw_inject_js() {
   //quickie stand-in for passing JS text via wp_localize_script()
   $script = 'alert("Hello world!");';
   //the actual callback, still needs some kind of sanitation and user security
   echo '<script>', $script, '</script>';
}

    The main problem is you are limited in how much sanitation can be applied. Code input should be limited to logged in users who have unfiltered HTML capability. Accepting unqualified user’s code would be very irresponsible. This isn’t really any worse than users entering script tags and code in text widgets or post content.

    Dion

    (@diondesigns)

    Whether javascript or PHP, eval() is fine if you have control over what is being passed into it. If you don’t, then it can be dangerous.

    @bcworkz, hosting companies don’t disable eval() in PHP because it is used in almost all major packages — including WordPress.

    Thread Starter Yuval

    (@yuvalsabar)

    Thank you for your answers.
    The only one who can post a script is the admin of course, but anyway I will add a warning to the plugin about that issue.

    What about the ;\r\n issue? Is this ok to sanitize a script with sanitize_textarea_field and than run the string with ;\r\n, or there’s a better way?

    Moderator bcworkz

    (@bcworkz)

    A semi-colon (;) is the formal line terminator for JS language, you should not remove it. The \r\n should merely be 2 whitespace characters which is no different than space characters when interpreting JS. There’s no need to remove these characters, unless what you are seeing are actually Latin chars r & n preceded with separate ASCII backslash chars, string length 4. var_dump the string to confirm which you have.

    I’m not sure you can fully sanitize JS code without corrupting it, this is in part why it’s important to limit who can add it. However, you can still escape it prior to saving in the DB, but it must then be un-escaped upon retrieval prior to output. Not escaping output makes my skin crawl, but I don’t see how it could be done if JS is to be valid. If you are seeing the \r\n as 4 chars and not whitespace, the string probably was sanitized somehow, which now needs to be undone.

    DD – thanks for setting me straight. Must stop quoting crap I read on the Internet.

Viewing 4 replies - 1 through 4 (of 4 total)
  • The topic ‘Eval is evil?’ is closed to new replies.