Skip to content
This repository was archived by the owner on Dec 19, 2023. It is now read-only.

Security fix for Arbitrary Code Execution in telejson#2

Merged
huntr-helper merged 5 commits into418sec:masterfrom
zpbrent:patch-2
Apr 6, 2021
Merged

Security fix for Arbitrary Code Execution in telejson#2
huntr-helper merged 5 commits into418sec:masterfrom
zpbrent:patch-2

Conversation

@zpbrent
Copy link
Copy Markdown

@zpbrent zpbrent commented Mar 26, 2021

📊 Metadata *

The telejson.reviver() which is used to parse string data back to json structure can be abused to execute arbitrary code when the lazyEval option is set to false (i.e., disabled). The root cause is the attackers can purposely inject a bracket at the end of the function property (invoking IIFE), that may be stringified by telejson.replacer() or telejson.stringify(). Even worse, despite the default value of lazyEval option is set to true for telejson.parse(), the telejson.reviver() have that vaule as false by default.

Bounty URL: https://www.huntr.dev/bounties/1-npm-telejson/

⚙️ Description *

Sanitize the brackets at the end of the function property for the input string to telejson.reviver(), since the normal use of telejson.replacer() and telejson.stringify() cannot make brackets at the end of the function property for the json objects.

💻 Technical Description *

const sourceSanitized = source.replace(/[(\(\))|\\| |\]]*$/,'');
This fix has considered the bypass possibility such as multiple ( ) \ ] ` and spaces, in case they apper at the end. Any more?

🐛 Proof of Concept (PoC) *

// PoC.js
telejson=require('telejson');
str = '{"fn":"_function_fn|function () {require(\'child_process\').exec(\'touch HACKED\');}()"}';
JSON.parse(str, telejson.reviver({}), 2);

After running node PoC.js, the file HACKED can be illegally created.

🔥 Proof of Fix (PoF) *

// PoF.js
telejson=require('telejson');
str = '{"fn":"_function_fn|function () {require(\'child_process\').exec(\'touch HACKED\');}()"}';
JSON.parse(str, telejson.reviver({}), 2);

After running node PoC.js, the file HACKED cannot be created.

👍 User Acceptance Testing (UAT)

image

@huntr-helper
Copy link
Copy Markdown

👋 Hello, @ndelangen. @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!


@ndelangen - 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)

huntr

@ndelangen
Copy link
Copy Markdown

@zpbrent Do you want to re-create the PR, or should I merge this duplicated one?
storybookjs#60

@JamieSlome
Copy link
Copy Markdown

@ndelangen - if we can merge this one, we can make sure that @zpbrent gets the bounty rewards automatically.

Cheers! 🍰

@JamieSlome
Copy link
Copy Markdown

@ndelangen - you just need to write the LGTM in a new comment below.

@ndelangen
Copy link
Copy Markdown

LGTM

@JamieSlome
Copy link
Copy Markdown

@huntr-helper - LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants