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

Migrate to vm2 instead of safer-eval #16

Closed
dozoisch opened this issue Jan 7, 2020 · 5 comments
Closed

Migrate to vm2 instead of safer-eval #16

dozoisch opened this issue Jan 7, 2020 · 5 comments
Assignees

Comments

@dozoisch
Copy link

dozoisch commented Jan 7, 2020

Hi!

safer-eval is now considered unsafe.

A safer option would be vm2. This only requires changes in executeJavascript which shouldn't be too bad.

I'm trying to get a PR up in the next two weeks.

@imlucas imlucas self-assigned this Jan 8, 2020
@imlucas
Copy link
Contributor

imlucas commented Jan 8, 2020

That would be very much appreciated! I'll finish #17 tmrw for all of the other issues

imlucas added a commit that referenced this issue Jan 10, 2020
@rueckstiess
Copy link
Member

Hi @dozoisch, we're taking a slightly different direction with this, as vm2 is not available in the browser context and there could be potential security implications as well.

Instead, we have created a new package ejson-shell-parser which creates an AST from the query string, validates every node in the syntax tree against a whitelist of specifically allowed javascript features, and then constructs the query by walking the tree.

This is a safer option and works both for node and the browser. We have just released a release candidate 1.6.0-rc.0 to collect feedback on this approach from downstream teams. Please see the PR #22 with some limitations that we are going to impose on the query parser, and feel free to comment with any feedback on that PR as well.

@dozoisch
Copy link
Author

@rueckstiess that's awesome! I'll try it in mongo-express.

@JLLeitschuh
Copy link

@rueckstiess you may need a CVE given that using safer-eval would have allowed an attacker to achieve RCE against for users of your library.

CC:

@rueckstiess please let me know if I'm misunderstanding something.

@rueckstiess
Copy link
Member

rueckstiess commented Jan 28, 2020

Hi @JLLeitschuh there is already an existing CVE for the safer-eval module describing this issue: https://nvd.nist.gov/vuln/detail/CVE-2019-10769

Also, since the new parser behaves slightly differently in certain cases (i.e. we disallow the use of function definitions and other "risky" Javascript expressions in the query language) this constitutes a backwards breaking change, so we decided to release this as a new major version 2.0.0.

We also wrote a migration guide explaining the breaking changes in more detail.
https://github.com/mongodb-js/query-parser/blob/master/MIGRATION.md

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

No branches or pull requests

4 participants