645440 - static analysis should detect and reject due to innerHTML

Status: RESOLVED DUPLICATE of bug 633888

(addons.mozilla.org Graveyard :: Developer Pages, defect)

Description

[Dietrich Ayala (:dietrich)]

My add-on was rejected with this reason:

<snip>
Your version was rejected because of the following problems:

1) Your add-on creates DOM nodes with raw HTML strings containing unsanitized string data. While the recommended method of creating DOM nodes is to use JavaScript DOM building methods such as createElement and appendChild (see https://developer.mozilla.org/en/How_to_create_a_DOM_tree) or one of the libraries which simplify using this method, creating content via strings is allowed if non-static data is sanitized with a function such as the following:

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

I assume that's because I assign to innerHTML in my content script?

If that's the case, then filing this bug because:

1. If it's innerHTML that triggers this, then the rejection text should mention innerHTML.

2. If this is a reject-able problem, the static analysis that occurs at add-on upload time should detect innerHTML and reject it.

3. All strings sourced in the innerHTML assignment are from inside Firefox itself - from labels on application menus. These should be safe... hm, unless there's a malicious add-on that creates a menu item in Firefox that contains evil markup specifically tailored for this case. So never mind, I guess we could be extra-defensive here.

Comment 1

[Jorge Villalobos [:jorgev] (he/him)]

Coincidentally (or maybe not), this is something that is currently being worked on.