Arbitrary Code Execution in telejson.reviver() with lazyEval disabled

[zpbrent]

Describe the bug

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.

Steps to reproduce the behavior

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

Expected behavior

the file HACKED should not be created.

Screenshots and/or logs

Environment

• OS: [ubuntu]
• Node.js version: [v10.16.0]
• NPM version: [7.6.3]
• Browser (if applicable): [N/A]
• Browser version (if applicable): [N/A]
• Device (if applicable): [N/A]

Additional context

I have opened a PR (418sec#2) which will fix the bug in telejson. Please take a review.

If you are fine with that fix, please comment @huntr-helper - LGTM at 418sec#2, or if you need any modifications, please also comment on that PR. Thanks.

Ref: 418sec#2

[yoshino-s]

Actually, if we want to transfer an function from one to another exactlly same, we have no solution to fix the arbitrary code execution when attack can control the input of reviver.

If we add vm2 or other vm package, it may change the function behavior.

But what i think is that this should not be our responsibility to prevent this type of vuln. Just alert users to do not use this with user input is a better solution.

Or we can add an options which can disable function parse.