[FIX] XSS validating context and encoding HTML

[Mik317]

📊 Metadata *

Please enter the direct URL for this bounty on huntr.dev. This is compulsory and will help us process your bounty submission quicker.

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

⚙️ Description *

The form library suffered of a XSS issue, which was caused by 2 minor issues inside the code, which made possible the usage of eval on unsanitized values (inside the "override" of parseJSON) and html parsing on a unsanitized AJAX response.

💻 Technical Description *

The 2 issues have been fixed in the following way:

• The eval inside the parseJSON function has been removed, while it's been added a error which arises when the default $.parseJSON function (on jquery) isn't declared (anyone with good intentions would simply add the jquery script on the page and all works correctly again).

• The unsanitized AJAX response was previously passed to parseHTML without any check, making possible inject additional HTML. I used a peculiarity of jquery to translate the HTML nodes evaluated into text nodes, which are equal to HTML encoded entities (can be verified seeing this:
  )

🐛 Proof of Concept (PoC) *

No PoC was provided, so I worked mostly theoretically on the issue/lines identified by the 2 issues in the original repo

🔥 Proof of Fix (PoF) *

Theoretical fix 😄

👍 User Acceptance Testing (UAT)

Can't be sure of this but seems all OK (nodes are still nodes of different type and a function is null --> arises exception due to a function undefined)

[toufik-airane]

[mufeedvh]

JQuery.parseHTML() is not a good mitigation for XSS vulnerabilities, it's bypassable. The fix also doesn't seem well fit in the codebase, you rewrote the parseJSON function to throw a console error which initially just returned a new window.

Good approach tho, without having a PoC! 👏

📚 References:

• https://bugs.jquery.com/ticket/11974
• https://stackoverflow.com/questions/53427381/is-using-jquery-parsehtml-to-remove-script-tags-enough-to-prevent-xss-attacks

[Mik317]

Hi @mufeedvh 😄
Thanks for making me aware of the possible issues, however I think the fix makes impossible the XSS:

• The $.parseHTML function was used insecurely directly on the data variable, which could contain the malicious content and therefore be evaluated, leading to XSS
  . However, the $('<div>').text(data).html() code returns always a text node, which can't be evaluated as an image or in different dangerous ways (like embed or frame object which could be evaluated) , making impossible the html injection and encoding the data variable as html entities
  
  .

A different solution was using document.createTextNode which is 💯 safe from XSS, however it's impossible make the new node from the HTML made since using this method, the new node isn't created:
(as far as I can understand, the html is supposed to be parsed safely, so this would make impossible process the new node).

I checked if the fix was broken also making a function like this:

function escapeHtml(unsafe) {
    return unsafe
         .replace(/&/g, "&")
         .replace(/</g, "&lt;")
         .replace(/>/g, "&gt;")
         .replace(/"/g, "&quot;")
         .replace(/'/g, "&#039;");
 }

and evaluating the passed data and parsing it through parseHTML gives the same output (without execution as in the fix applied):

• The parseJSON function was returning the evaluation of eval('('+s+')') and this is a good problem since I'm not able to understand if s was a function which returned a value or it's a basic command/void function which doesn't return nothing (in that case, the null would be ok since alert(1) === null
  .

Let me know if the part regarding parseHTML is enough secure in your opinion and it would be awesome know what's exactly the return type of the s variable 😄

Cheers,
Mik

[mufeedvh]

A good approach to the fix but I do feel like it's a theoretical one tho, so I am not sure if this will fix the XSS or not, I couldn't get the PoC to validate it.

[JamieSlome]

@mufeedvh - just to confirm, are you happy with this fix?

[JamieSlome]

Attaching reference (jquery-form#580)! 🍰

@kevindb - we will look to open this pull request now!

[huntr-helper]

Congratulations Mik317 - 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!