patriksimek / vm2

Advanced vm/sandbox for Node.js
MIT License
3.86k stars 293 forks source link

URL.search is readonly #500

Closed garymathews closed 1 year ago

garymathews commented 1 year ago

I found that attempting to set the search property of a URL object does not work due to being forced as readonly. This property should be writable (see https://nodejs.org/api/url.html#urlsearch)

'use strict';
const { URL } = require('url');
const url = new URL('https://google.com');

url.search = '?query=vm2';
console.log('url', url);
node:internal/process/esm_loader:97
    internalBinding('errors').triggerUncaughtException(
                              ^
TypeError [Error]: 'set' on proxy: trap returned falsish for property 'search'

Possible solution; not sure if this is safe?

lib/resolver-compat.js

function defaultBuiltinLoaderUrl(resolver, vm, id) {
    const url = resolver.hostRequire(id);
    return vm.readonly({
        ...url,
        URL: vm.protect(url.URL)
    });
}

const SPECIAL_MODULES = {
    ...
    url(vm) {
        return defaultBuiltinLoaderUrl;
    }
XmiliaH commented 1 year ago

I am not certain to add this as there might be other instances where properties should be able to be changed and maintaining this list is cumbersome.

As a workaround it is possible to expose the url.URL into the sandbox before it is required. The access type is given the first time the object is exposed. In this case URL will only be accessible as protected for the runtime of the sandbox.

However, care should be taken with protect as it allows for prototype pollution.

const vm = new NodeVM(...);
vm.protect(url.URL);
vm.run('your code');