asvd / jailed

execute untrusted code with custom permissions
MIT License
1k stars 73 forks source link

Node dies with unhandled exception on jailed plugin syntax error #24

Closed cristiano-belloni closed 8 years ago

cristiano-belloni commented 8 years ago

I'm using

pluginInstance = new jailed.DynamicPlugin(script, api)

and

pluginInstance.whenFailed(function() {
   // Handle plugin failing
 })

but, if script has a syntax error, the process dies with an unhandled exception:

SyntaxError: Unexpected token }
    at Object.exports.runInNewContext (vm.js:48:16)
    at executeJailed (/myDir/node_modules/jailed/lib/_pluginNode.js:194:12)
    at execute (/myDir/node_modules/jailed/lib/_pluginNode.js:143:5)
    at process.<anonymous> (/myDir/node_modules/jailed/lib/_pluginNode.js:36:9)
    at emitTwo (events.js:100:13)
    at process.emit (events.js:185:7)
    at handleMessage (internal/child_process.js:689:10)
    at Pipe.channel.onread (internal/child_process.js:440:11)

Is there a way to handle this without the process dying?

cristiano-belloni commented 8 years ago

(please note that the plugin is submitted by users, and can have a syntax error)

asvd commented 8 years ago

Would it help, if you execute user's code in a try-catch block?

Have a look on how it's implemented in this demo and how it reacts to a syntax error in a user's provided code:

http://asvd.github.io/jailed/demos/web/console/

cristiano-belloni commented 8 years ago

The problem is that I need to call functions on the user code / plugin . Evaluating the user block won't work.

Also, if you specify, for example function(a) { return 'a' } in the Web Console, it returns Unexpected token (

asvd commented 8 years ago

But can you maybe call those functions in try-catch block? What would happen then?

Also, if you specify, for example function(a) { return 'a' } in the Web Console, it returns Unexpected token (

Same happens if you run that code in Chrome Dev-tools. To make it work, you can add an assignment:

b = function(a) { return 'a' }

cristiano-belloni commented 8 years ago

(what I'm doing now is something like):

var log
  , except
  , action
  , next
  , parameters = {}
  , staticData = {}
  , initException = null
  , indexedDB = null
  , location = null
  , navigator = null
  , onerror = null
  , onmessage = null
  , performance = null
  , self = null
  , webkitIndexedDB = null
  , postMessage = null
  , close = null
  , openDatabase = null
  , openDatabaseSync = null
  , webkitRequestFileSystem = null
  , webkitRequestFileSystemSync = null
  , webkitResolveLocalFileSystemSyncURL = null
  , webkitResolveLocalFileSystemURL = null
  , addEventListener = null
  , dispatchEvent = null
  , removeEventListener = null
  , dump = null
  , onoffline = null
  , ononline = null
  , importScripts = null
  , console = null

try {
    application.whenConnected(function() {

      application.remote.start()

      log = application.remote.log
      action = application.remote.action
      except = application.remote.except
      next = application.remote.next

      if (initException) except({type: 'init', e: stringifyException(initException)})

      application.setInterface ({
        callUserFunction: function(data) {
          try {
            onUserFunction(data)
            next()
          }
          catch(e) {
            except({type: 'runtime', e: stringifyException(e)})
          }
        }
      })
      ${script}

    })

}
catch(e) {
  initException = e
  if (except) except({type: 'init', e: stringifyException(e)})
}

var stringifyException = function(err, filter, space) {
  var plainObject = {}
  Object.getOwnPropertyNames(err).forEach(function(key) {
    plainObject[key] = err[key]
  })
  return JSON.stringify(plainObject, filter, space)
}

where ${script} contains the user code, which is like:

function onUserFunction (data) {
  // do something with the data
  log('did something with the data')
}
cristiano-belloni commented 8 years ago

@asvd - btw, if I execute the users's code in a try catch block:

 try {
        ${script}
      }
    catch (e) {
      initException = e
      if (except) except({type: 'init', e: stringifyException(e)}
    }

It still dies with an unhandled exception

asvd commented 8 years ago

so user code is substituted into the plugin code body before creating the plugin instance?

cristiano-belloni commented 8 years ago

Yep, it is.

cristiano-belloni commented 8 years ago

I just can't pass it as a string and then eval it, because I need to call a callback in the user code and it needs to call callbacks in mine.

asvd commented 8 years ago

Not good, user can close the brace and break the structure of the code.

Do the following:

Is there any callbacks you cannot call in this way?

asvd commented 8 years ago

Should work if user's code simply calls log(), since eval() executes code in a current context and has everything available.

But if user-provided code is only intended to process some data, I would suggest simply to let user return the result, and then perform any respective actions on the application site. Why do you need user's code to do something additional?

asvd commented 8 years ago

See another demo concerning processing the data with user-provided code:

http://asvd.github.io/jailed/demos/web/process/

cristiano-belloni commented 8 years ago

Because the user has to send commands to the main application based on the data. Ideally, the user would only have to write something like:

var state = {}

var initialize = function(initData) {
  // This is called once
  state.whatever = initData.whatever
}

var onData = function(data) {
  // This is called repeatedly every time there's a new piece of data
  state.whatever = state + data.whatever
  log('did something with the data')
  action('DO_SOMETHING', calculatedValue)
}
asvd commented 8 years ago

You can also return a string describing which respective action to perform on the application site. Or an array of them in case there might be several actions to be performed. Or a serialized object with properties containing the detailed description concerning what exactly user code wants to happen.

asvd commented 8 years ago

Your example:

var onData = function(data, oldState) {
    return {
        newState: oldState + data.whatever,
        action : {
            name: 'DO_SOMETHING',
            params: [calculatedValue]
        },
        log : 'did something with the data'
    };
}
cristiano-belloni commented 8 years ago

I'll try something similar, thank you. Btw, the last demo doesn't work -- Mixed Content: The page at 'https://asvd.github.io/jailed/lib/_frame.html' was loaded over HTTPS, but requested an insecure Worker script 'blob:null/4abedc87-2d97-45be-9fc6-1c9bc351aa60'. This request has been blocked; the content must be served over HTTPS.

asvd commented 8 years ago

yep, will investigate on this. Should work if you load it by http instead of https

cristiano-belloni commented 8 years ago

@asvd Can I close this or you need it open for the https issue?

asvd commented 8 years ago

We can close it. The https issue is solved on HEAD already.