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

The jsonpath package is insecure (code execution + prototype pollution) #110

Open
cykoder opened this issue Feb 17, 2023 · 3 comments
Open

Comments

@cykoder
Copy link

cykoder commented Feb 17, 2023

This library uses jsonpath under the hood, which makes sense but this library is not secure. Filter expression can lead to arbitrary code execution (albeit, due to a coincidence most common vectors wont work. technically its a bug in jsonpath) or - the one that is definitely active in that library atm - prototype pollution. It is difficult to filter all the possible ways of doing that, and even though it uses static evaluation to run the filters the static-eval library is not secure or intended to be as the author states.

For this reason I'd advise to look for alternatives to jsonpath - or if you can and need this PEX library now, run the pex methods in a complete sandbox where code execution and prototype pollution arent as big of a concern. I've yet to find a jsonpath library on NPM (forks of jsonpath + jsonpath-plus all have these vectors).

If for example this library is run on a server, that can be a serious security issue. At the very least it can cause denial of service using a simple payload. This issue is known and is discussed briefly in the static-eval repo issues

@cykoder cykoder changed the title The jsonpath package is insecure The jsonpath package is insecure (code execution + prototype pollution) Feb 18, 2023
@TimoGlastra
Copy link
Contributor

There was a similar issue opened in the PEX spec (decentralized-identity/presentation-exchange#398), and it was raised that https://www.npmjs.com/package/@astronautlabs/jsonpath isn't impacted by this issue (at least the arbitrary code execution, I'm not sure on prototype pollution).

@cykoder
Copy link
Author

cykoder commented Jun 6, 2023

I'm not sure on prototype pollution

Reference my comment in that thread: decentralized-identity/presentation-exchange#398 (comment) - it is vulnerable as it uses static-eval. It's a low-change fork from jsonpath afaik

@nklomp
Copy link
Contributor

nklomp commented Jul 10, 2023

Although a bit late. Thanks for the ticket.

In the next minor release we will update to @astronautelabs/jsonpath, as static-eval is better than current jsonpath approach.

At the same time we are also working on a more permanent solution looking to sandbox parts of the code together with jsonpathplus, which allows you to disable script execution/evaluation altogether

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

3 participants