Security fix for Arbitrary Code Execution in json-ptr

[zpbrent]

📊 Metadata *

JsonPointer.get that is designed to get the target object's value at the pointer's location is vulnerable to arbitrary code injection and exection, mainly due to the lack of sanitizing for user's inputs of the pointer's location.

Bounty URL: https://www.huntr.dev/bounties/2-npm-json-ptr/

⚙️ Description *

Escape ' to avoid the injection.

💻 Technical Description *

Use path[i].replace('\\', '\\\\').replace(/\'/g, '\\\'') to escape ' to avoid the injection.

🐛 Proof of Concept (PoC) *

jptr=require('json-ptr');
JsonPointer=jptr.JsonPointer;
JsonPointer.get({}, '/aaa\'\]\)\) !== \'undefined\') \{return it;\}; console.log(\'HACKED\'); if((([\'a'); // HACKED

🔥 Proof of Fix (PoF) *

👍 User Acceptance Testing (UAT)

[huntr-helper]

👋 Hello, @treybrisbane. @zpbrent has opened a PR to us with a fix for a potential vulnerability in your repository. To view the vulnerability, please refer to the bounty URL in the first comment, above. If you want this fix in your repository, a PR will automatically open once you comment:

@huntr-helper - LGTM

---

☎️ Need further support?

Come and join us on our community Discord!

---

❓@treybrisbane - want more fixes like this?

Copy this snippet into your README.md for more vulnerability fixes in the future:

[![huntr](https://cdn.huntr.dev/huntr_security_badge_mono.svg)](https://huntr.dev)

[elimumford]

path[i] is a PathSegment which is string | number, so this code violates the TypeScript checks.
It also drastically changes the call stack as this replaces the replace function call that is a local function, not string.replace

Alternative solution:
return body + " && \n\ttypeof((it = it['" + replace(replace(path[i] + '', '\\', '\\\\'), '\'', '\\\'') + "'])) !== 'undefined'";

[cerebralkungfu]

Thanks for the PR.

I know there is a Huntr bounty system of some sort related to this PR so I'll try to respect that process even though I don't know what the process is.

@elimumford correctly points out that your fix works in Javascript but violates the project's typings. Thanks Eli. Please address the typings... Eli's alternative looks good.

I try not to accept PRs without tests that 1) exploit the bug, 2) prove it is fixed, and 3) ensure there isn't a regression later.

Your PoC, PoF, and UAT seem to indicate you are willing to do the work. If so, make it so -- otherwise acceptance will be delayed until I get time to do it myself.

[cerebralkungfu]

For anyone who comes across this PR or the associated vulnerability on Huntr or elsewhere. This is a legit and valid vulnerability that will be patched. In the mean time, you can mitigate the vulnerability in your own code, just like when you work with a database, by never passing un-sanitized user input to any of the methods exposed by the json-ptr library.

[cerebralkungfu]

I went ahead applied the fix for flitbit#30 (Eli's alternative), which also fixes flitbit#28, supplied the missing tests, and I put it in a branch in the underlying repo, which was slightly ahead of your fork (I had already updated dependencies to address other security warnings).

To preserve the PR and probably somebody's bounty (whatnot), merge it here and I'll accept the PR.

If you have an alternate approach I'm all ears... but keep it light... the day job is calling.

[JamieSlome]

@cerebralkungfu - if you could visit https://www.huntr.dev/bounties/2-npm-json-ptr/ and just validate the advisory on our platform, that would be amazing - it is just a click of a button. This will also reward the reporter for their work! 🍰

[cerebralkungfu]

Um, yeah...

Looks like I'll have to come back to it. In the mean time, I'm publishing the patch from our own branch.

[JamieSlome]

@cerebralkungfu - apologies for this.

It looks like our automation didn't pick up on the fact that you are a maintainer of the repository. I have added you now, and the validation will work.

Cheers!

[cerebralkungfu]

Of course, I broke the inter-tubes! I converted my personal account @flitbit to an organization last fall and revived an even older, unused, personal account that was sitting around. I use the same email address for my new/old @cerebralkungfu account as I did for @flitbit previously. As an unintended consequence, I lost any reputation I may have had as an individual; no surprise your system wouldn't identify me as an author.

Its amazing how many GitHub integrated systems break on this [seemingly] simple edge case.

[cerebralkungfu]

Fixed in v2.1.0 and above.

[JamieSlome]

Awesome, thanks for the help @cerebralkungfu!

Would love to get some feedback on our process if you have any, otherwise, have a great weekend!