search

Sphereon-Opensource / PEX

A Typescript implementation of the DIF Presentation Exchange specification.
Apache License 2.0
39 stars 16 forks source link

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

Open cykoder opened 1 year ago

cykoder commented 1 year ago

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

TimoGlastra commented 1 year ago

There was a similar issue opened in the PEX spec (https://github.com/decentralized-identity/presentation-exchange/issues/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 commented 1 year ago

I'm not sure on prototype pollution

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

nklomp commented 1 year ago

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