Get rid of jQuery .html() or .append() with Handlebars and Backbone

Add-ons Development
#1

Hi Mozilla team,

I have a Mozilla Addon - Requestly. I have ported it from Chrome Extension. Most of the code is same with minor tweaks. I have submitted it for review here. But It has been rejected with the following reasons:

  1. Unsafe eval from remote Url - Accepted
  2. Code uses jQuery html/append or something similar

Actually my app/addon is based on Backbone+Handlebars. The way I use handlebars is generate the markup with Handlebars compile and then passing a json object. Then I append the markup as innerHTML/jQuery.html of existing DOM Node.

Based on different routes, I render different views in the same element thus calling it as single page app.

Can somebody please let me know how can I get this approach approved with Mozilla Addon Reviewer team. I am also open to fix anything in my implementation if there needs be.

Please also note that current code is also working on Chrome Store from couple of years and it also passed their review mechanism.

#2

The Chrome Store has no review mechanism. So there was nothing to pass.

I have an add-on myself which loads markup and assings it to an elements .innerHTML.

To get this accepted by the reviewers you have to make it script injection impossible, and you should do so in a way that is easy to understaind. In my case that is:

element.innerHTML = sanatize(potentiallyUnsafeHtml);


/**
 * Removes any tags (not their content) that are not listed in 'allowed' and any attributes except for href (not data: or javascript:) and title (order must be href, title)
 * NOTE: Is only secure to assign this directly to .innerHTML or .outerHTML, any concatination sanatize(s1) + sanatize(s2) or passing it to docuent.write() may still be unsafe
 * @param  {string}  html  Untrusted HTML markup
 * @return {[type]}        Sanitized, undangerous, simple HTML
 * @license          CC-by 4.0
 * @author           Niklas Gollenstede
 */
function sanatize(html) {
    const allowed = /^(?:a|b|big|br|code|div|i|p|pre|li|ol|ul|span|sup|sub|tt|math|semantics|annotation(?:-xml)?|m(?:enclose|error|fenced|frac|i|n|o|over|padded|root|row|s|space|sqrt|sub|supsubsup|table|td|text|tr|under|underover))$/;
    return html.replace(
        (/<(\/?)(\w+)[^>]*?(\s+href="(?!(?:javascript|data):)[^"]*?")?(\s+title="[^"]*?")?[^>]*?>/g),
        (match, slash, tag, href, title) => allowed.test(tag) ? ('<'+ slash + tag + (title || '') + (href || '') +'>') : ''
    );
}
1 Like
#3

Thanks @NilkasG. Let me add this sanitization to my code as well.
Thanks for your help.

#4

You are welcome. Feel free to do so.
I’ve edited the header to include a license and a NOTE on the security, and have also removed the target="_blank" insertion.

#5

@NilkasG Thanks man!!

So Can I use your code as is now including license. Is that right ?
Thanks once again!!

#6

Hey sachin,
I am also looking for a sanitizer which passes the FF addon review panel. Does adding this sanitizer worked for u ?

#7

This one is approved by the AMO reviewers -

function escapeHTML(str) { return str.replace(/[&"'<>]/g, (m) => ({ "&": "&amp;", '"': "&quot;", "'": "&#39;", "<": "&lt;", ">": "&gt;" })[m]); }

https://developer.mozilla.org/en-US/Add-ons/Overlay_Extensions/XUL_School/DOM_Building_and_HTML_Insertion#innerHTML_with_HTML_Escaping

#8

@noitidart @jorgev Thanks for the quick reply. The link that you pointed basically does a html encode and would render html as a text. I am more interested in loading remote html safely (see the last section). It talks about using nsIParserUtils.parseFragment() .

I have some question regarding the usage of nsIParserUtils.parseFragment(). Can we use nsIParserUtils.parseFragment() from Webextension. As per the documentation it seems this can used only via legacy ADD-ON SDK flow.

Is there an equivalent of nsIParserUtils.parseFragment() API which can be used in WebExtension flow.

#9

Nope. As a general rule, if it starts with nsI you can’t.

Is there an equivalent of nsIParserUtils.parseFragment()

Nope. You will have to bring your own sanitizer.
If the output of the sanatize function above still contains enough information, you can use that. It is quite performant (I don’t think one can do much better) and works.

Otherwise, if you use XMLHttpRequests to get your markup, you could set it responseType to document and selectively “clone” the returned DOM.
(I don’t think XMLHttpRequest evaluates any scripts when building the DOM, but I am not sure about that.)

If that doesn’t work for you either, I’d suggest https://www.npmjs.com/package/htmlparser2.

#10

nipun,

were you able to get your add on pass review with https://www.npmjs.com/package/htmlparser2. Please do let me know, i am having the same issue.