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

[library] quickjs-emscripten-sync - easily pass objects between host and guest #39

Open
rot1024 opened this issue Jun 10, 2021 · 2 comments
Labels
community Community discussion or information

Comments

@rot1024
Copy link

rot1024 commented Jun 10, 2021

Thank you for creating this wonderful library.

I have developed "quickjs-emscripten-sync" to facilitate the passing of objects between QuickJS VMs and hosts.

https://github.com/reearth/quickjs-emscripten-sync

It is using a limited API to exchange objects, which may not be efficient, but It is currently able to pass objects smoothly.

In the TODO of quickjs-emscripten, it says that they are planning to support binding of classes and other objects. This implementation may be useful at that time.

@justjake
Copy link
Owner

@rot1024 This is awesome! Thank you for sharing.

I skimmed the code briefly and didn't see anything suspect (although without comments I didn't understand VMMap at first glance).

Great job on the README. I really like that you included security warnings and example.

In the TODO of quickjs-emscripten, it says that they are planning to support binding of classes and other objects. This implementation may be useful at that time.

My requirement to include buinding in quickjs-emscripten directly is that the binding API requires consumers to consider security. It's a tricky design question. Ideally, the approach would use an allow-list, with only primitive values allowed by default. It would be up to the consumer to specify the properties of objects that could be marshalled or synced.

Your approach with isMarshallable is maximally configurable which is great, but the default vulnerability of the approach means it doesn't satisfy my requirement for this feature. I also don't understand the security implications of syncing eg Symbol.prototype but I worry that this means that by default any code executed inside a quickjs-emscripten-sync VM can attack the host's built-in objects directly.

@rot1024
Copy link
Author

rot1024 commented Jun 14, 2021

Thank you for reply.

Yes, security is a top priority. It would certainly be beneficial to impose a strict security policy by default. I'd like to modify it that way soon (for example, it could be serialized to JSON by default, and allowed to be exchanged through proxies in a whitelist).

I also don't understand the security implications of syncing eg Symbol.prototype but I worry that this means that by default any code executed inside a quickjs-emscripten-sync VM can attack the host's built-in objects directly.

In fact, I don't think Symbol.prototype, etc. are dangerous since they are not synchronizing, but just mapping the host-side world to the VM-side world (which is beneficial to avoid prototype pollution). For example, if an Array.prototype is about to be marshaled from the host side, instead of creating a proxy for it, it will simply return the Array.prototype from the VM side. The same goes for the others. However, we may need to look for potential dangerous cases.

Most of all, I think disabling marshalling by default and allowing it in a whitelist would be most beneficial to increase safety.

Thank you very much for your useful feedback and I look forward to seeing further evolution of quickjs-emscripten in the future.

@justjake justjake changed the title I have developed quickjs-emscripten-sync [library] quickjs-emscripten-sync - easily pass objects between host and guest Feb 18, 2023
@justjake justjake added the community Community discussion or information label Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community discussion or information
Projects
None yet
Development

No branches or pull requests

2 participants