Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[investigation] what other intrinsics should be remapped #61

Open
caridy opened this issue Jan 25, 2020 · 3 comments
Open

[investigation] what other intrinsics should be remapped #61

caridy opened this issue Jan 25, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@caridy
Copy link
Contributor

caridy commented Jan 25, 2020

Today, the following list is remapped:

[
    'Object',
    'Function',
    'URIError',
    'TypeError',
    'SyntaxError',
    'ReferenceError',
    'RangeError',
    'EvalError',
    'Error',
]

But there are a lot more pieces that can be remapped safety. Additionally we have issues with intrinsic accessors accessing internal slots that will not work with the remapping of the intrinsic prototypes, which might requires a lot more patching on the intrinsics in the sandbox before using the sandbox.

@caridy caridy added the enhancement New feature or request label Jan 25, 2020
@caridy
Copy link
Contributor Author

caridy commented Jan 25, 2020

Some browses are using internal slots for Errors (Firefox, we are looking at you), which might interfere with the remapping mechanism because some accessors might through when an error is passed from outer realm into sandbox (think of Promise.catch maybe). We should validate this.

@mmis1000
Copy link

mmis1000 commented Feb 21, 2020

It looks like things like Date.prototype.getTime() Map.prototype.get can't be remapped safely unless you remap the methods and re-dispatch based on where they are object of remote realm or object of current realm.

https://github.com/mmis1000/secure-ecmascript-sandbox/blob/4afbe936ec5e309c6d56ac9758e2a4139d07d8a2/src/browserRealm.ts#L855-L863

but that has another problem.
unless you also remap the methods in outer realm.
Date/Map and other object that relies on internal slot created in inner realm won't work in outer realm.

Some class I found that has method/getter relies on internal slot

https://github.com/mmis1000/secure-ecmascript-sandbox/blob/4afbe936ec5e309c6d56ac9758e2a4139d07d8a2/src/browserRealm.ts#L87-L120

These won't work unless you remap the methods.

In turns, it means these need to be whitelisted as this object when calling methods of these class. But that sounds dangerous to me though.

@caridy
Copy link
Contributor Author

caridy commented Apr 2, 2020

I have identified another small list of intrinsics that are present in node 12.x when creating a new VM:

  [ 'BigUint64Array', 'BigInt64Array', 'BigInt', 'WebAssembly' ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants