Fixed Cross-site Scripting

[mufeedvh]

📊 Metadata *

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

⚙️ Description *

The project x-editable validated JSON string with poorly crafted Regex making it easy to bypass the validation and inject JavaScript code to achieve XSS (Cross-site Scripting).

💻 Technical Description *

The implemented Regex to validate JSON was so poor that it only checks whether if the string begins and ends with [ or { and their counterparts ] or }. This means you can give it a string such as []+alert(1)+[] and it will match.

Come on man why would you write a whole Regex to validate JSON semantics lol, and JavaScript's Regex match doesn't have certain expressions implemented making it even harder.

So how do we fix it? Just parse the JSON with the JavaScript function JSON.parse() and if it fails, it's not a valid JSON string.

🐛 Proof of Concept (PoC) *

Refer: vitalets#1130

Payload used to bypass the current validation:

[]+alert(1)+[]

🔥 Proof of Fix (PoF) *

Parse the input JSON string and if it fails, it wouldn't proceed dynamically adding it to the HTML code preventing the XSS (Cross-site Scripting) vulnerability.

if (typeof s === 'string' && s.length) {
    try {
        JSON.parse(s);
        if (safe) {
            try {
                /*jslint evil: true*/
                s = (new Function('return ' + s))();
                /*jslint evil: false*/
            } catch (e) {} finally {
                return s;
            }
        } else {
            /*jslint evil: true*/
            s = (new Function('return ' + s))();
            /*jslint evil: false*/
        }
        return s;
    } catch (e) {
        // pass
    }
}

👍 User Acceptance Testing (UAT)

Just added a try/catch block to check for JSON.parse() fails.

[Mik317]

LGTM 😄 🎉

Just a note:
The author of the issue mentioned

For my site I opted to replace the new Function() call with a JSON.parse() call. This fix won't work for everybody though, as JSON.parse() is much stricter about what is and isn't allowed in a JSON string (e.g. all properties must be surrounded in double quotes and no trailing commas are allowed).

In case other will see this, I think it shouldn't anyway comport a breaking change since also the name of the function (tryParserJSON) makes clear the input should be a valid JSON, so there shouldn't be errors if people using this module had used it in the right way 😄

Cheers,
Mik

[mufeedvh]

In case other will see this, I think it shouldn't anyway comport a breaking change since also the name of the function (tryParserJSON) makes clear the input should be a valid JSON, so there shouldn't be errors if people using this module had used it in the right way 😄

@Mik317 Yep exactly, the whole purpose of the function is to parse a valid JSON string, JSON.parse() / the fix does exactly what the function is intended to. It halts if the input is not a valid JSON, as it should. 👍

[huntr-helper]

Congratulations mufeedvh - your fix has been selected! 🎉

Thanks for being part of the community & helping secure the world's open source code.
If you have any questions, please respond in the comments section. Your bounty is on its way - keep hunting!