JSONpath has security issues

[danielfett]

In PE, JSONpath is used both in the presentation definition as well as in the presentation submissions. In other words, both Holders and Verifiers need to execute JSONpath expressions in order to process PE. Usually, Verifier and Holder are not the same entity and should not fully trust each other.

JSONpath, however, intentionally allows for arbitrary script execution - you can't just pull it into your spec and expect people to create secure implementations. By default, it can enable exfiltration of security and privacy sensitive data, denial of service attacks, and server-side request forgery, to name just a few. I collected a couple of examples in this twitter thread: https://twitter.com/dfett42/status/1608433806964580352

Therefore, PE must ensure that JSONpath cannot be used to execute arbitrary scripts on a server, for example, by only allowing a reduced subset of JSONpath's syntax.

[csuwildcat]

This is not an accurate characterization of JSONPath. The JSONPath the spec does not allow arbitrary code execution, it even explicitly describes how implementers are to evaluate the syntax without engaging scripting engines: https://ietf-wg-jsonpath.github.io/draft-ietf-jsonpath-base/draft-ietf-jsonpath-base.html#name-attack-vectors-on-jsonpath-.

Some naïve Implementations of JSONPath certainly may defy the spec and eval scripts, but that's not the spec's fault. For example, here is a well-done JSONPath implementation that uses static evaluation to cover the features, as directed by the spec: https://github.com/astronautlabs/jsonpath#evaluating-script-expressions. (NPM package: https://www.npmjs.com/package/@astronautlabs/jsonpath)

[danielfett]

I understand that this is the original JSONPath proposal and predates the IETF draft by about 12 years. It explicitly mentions "script expression, using the underlying script engine". The IETF draft is a huge step forward in that it can be implemented without evaluating JS, but I doubt that many implementations follow this approach already.

[danielfett]

And to add, the spec currently references the 2007 JSONPath by Stefan Gössner, not the IETF draft. The only hint that is given to the reader is "via static evaluation" in the table in Section 11. IMO this security issue should be addressed and discussed, otherwise many implementations will be insecure.

[csuwildcat]

Would you be able to do a PR to add the pointers and extra text?

[dtmcg]

@danielfett nudge on the above

[Sakurann]

I think Daniel gave enough informaiton for the editors to do a PR that Daniel and the rest of us can review.
(PS please don't close this issue saying Daniel did not respond or did not do a PR - this security one is pretty important.)

[cykoder]

@csuwildcat it should be noted that the library you referenced uses static-eval under the hood. static-eval is still to this date open to arbitrary code execution exploits. heres one thats working as of now:

var evaluate = require('static-eval');
var parse = require('esprima').parse;

var src="(function (x) { return `${eval(\"console.log(global.process.mainModule.constructor._load('child_process').execSync('ls').toString())\")}` })()"
var ast = parse(src).body[0].expression;
evaluate(ast)

static-eval authors state that it shouldnt be used for untrusted input at all, other sandbox breakouts could be discovered in the future too. interestingly though, apparently the jsonpath library has a bug or coincidence in it that prevents certain known breakouts that can be easily executed in static-eval. the jsonpath library however is vulnerable to prototype pollution and shouldnt ideally be used

however - note that is known exploits. there could be others discovered in the future. a detailed blog post about the issue is here: https://licenciaparahackear.github.io/en/posts/bypassing-a-restrictive-js-sandbox/

this is primarily an issue with the npm ecosystem with current jsonpath libraries, which this implementation of PEX uses https://github.com/Sphereon-Opensource/PEX

[csuwildcat]

ACTION: link to the correct IETF spec for JSON Path: https://ietf-wg-jsonpath.github.io/draft-ietf-jsonpath-base/draft-ietf-jsonpath-base.html

[bumblefudge]

@csuwildcat will attempt a quick PR to triage this security issue in today's spec and we will deep-dive on guardrails in the coming implementation guide!

[Sakurann]

Other concerns I have heard are

1. what can be used as a "filter". For instance this example uses a regular expression for the filter pattern. Regular expressions are notorius for enabling DoS attacks (https://www.usenix.org/system/files/sec21-li-yeting.pdf)
2. Speficiation says that the filer can be a "JSON Schema descriptor" and supporting JSON schema as a filter option will create many security risks.