Overriding functions of objects from sandbox parameter inside NodeVM

[c53apirker]

Hello!

I was just working a bit with the vm2 package, and I came across a very surprising observation. Consider the following code example:

`myObject = { myvalue: 1 } myObject.isAdmin = function() { console.log("Hello from the outside function!"); return false; }
console.log(myObject.isAdmin())

const vm = new NodeVM({ console: 'inherit', sandbox: { myObject }, eval: false, wasm: false, require: { builtin: ['buffer', 'crypto', 'querystring', 'string_decoder', 'url', 'util'] } });

let javascriptString = 'myObject.isAdmin = function() { console.log("Hello from within the VM!"); return true; }'

functionScript = new VMScript( 'module.exports = async function () {\n' + javascriptString + '\n}' ).compile()

let functionInSandbox = vm.run(functionScript); functionInSandbox();

console.log(myObject.isAdmin())`

I pass in here an instance of myObject to the NodeVM of vm2, and within the VM i alter the function isAdmin to something different. When executing the above I observe the following output:

Hello from the outside function! false Hello from within the VM! true

I was wondering whether this behavior is expected? Since I could see from this quite severe security impacts, because it allows to override functions of instances passed in via the sandbox option. Consider a user object is passed with an isAdmin function, then the NodeVM would allow to override the isAdmin function of the object outside of the VM from within the VM.

I would have expected that the VM reverses the object's state change after the vm execution finishes, or that the entire assignment fails within the VM already (must be noted that eval is set to false also). Thank you for your considerations, and looking forward to your opinion on it.

Best Alex

[XmiliaH]

This is expected and wanted. If an object should not be altered from within the sandbox readonly can be used.

const vm = new NodeVM({
  console: 'inherit',
  eval: false,
  wasm: false,
  require: {
    builtin: ['buffer', 'crypto', 'querystring', 'string_decoder', 'url', 'util']
  }
});
vm.sandbox.myObject = vm.readonly(myObject);

[c53apirker]

Maybe i missed some but this:

` const myObject = { myvalue: 1 } myObject.isAdmin = function() { console.log("Hello from the outside function!"); return false; }
console.log(myObject.isAdmin())

const vm = new NodeVM({ console: 'inherit', sandbox: { myObject }, eval: false, wasm: false, require: { builtin: ['buffer', 'crypto', 'querystring', 'string_decoder', 'url', 'util'] } });
vm.sandbox.myObject = vm.readonly(myObject); let javascriptString = 'myObject.isAdmin = function() { console.log("Hello from within the VM!"); return true; }'

functionScript = new VMScript( 'module.exports = async function () {\n' + javascriptString + '\n}' ).compile()

let functionInSandbox = vm.run(functionScript); functionInSandbox();

console.log(myObject.isAdmin())`

leads to that output:

Hello from the outside function! false Hello from within the VM! true

[XmiliaH]

Yes, you need to delete then sandbox: { myObject }, line as it exposes the object into the sandbox and therefore the readonly call does not have an effect.