vm.runInContext is memory unsafe and can be used to dump core

[deian]

• Version: 6.5.0
• Platform:
• Subsystem: vm

vm.runInContext is memory unsafe and can be used to dump core.

This doesn't seem like a serious security vulnerability (hence my reporting here), but can certainly be used to cause DOS and it might be nice to have a stdlib that is memory safe.

const vm = require('vm');

const target = {a : 1337};

const handler = {
  getOwnPropertyDescriptor: (target, prop) => {
    throw 'w00t';
    // causes FromJust on line 128 of src/node_contextify.cc to dump core
  },
};

const sandbox = new Proxy(target, handler);


const context = new vm.createContext(sandbox);
const script = new vm.Script('');
script.runInContext(context);

Related to: #8539, #8538, #7902

• @mlfbrown

[imyller]

This is a known issue and affects most C++ API functions accepting non-primitive values.

For more information:

#7902 (comment)

[deian]

I don't think this fits under that category. (We're the ones that found #7902 FWIW) This is already using MaybeLocal; in fact the bug is using that to cause the crash

[bnoordhuis]

It's a variation on the same theme. CopyProperties() uses .FromJust() (which is basically success-or-die) when it should be using .FromMaybe() or .IsJust() and propagate pending exceptions.

[cjihrig]

@bnoordhuis thanks for hint. Proposed fix in #8649. Sorry if it is way off base.