Android Add-ons - Unsafe assignment to innerHTML

I have an addon that uses innerHTML
So naturally it cannot be hosted on Mozilla store.
So I host it myself and give instructions on how to install it - https://quicklook-606e5.firebaseapp.com/

For the current version of Firefox on Android this is no longer possible - by design.

Is there any future scenario where this limitation may eventually be relaxed?

I ask because my non-to-for profit addon has/had a genuine use-case for exploring publicly available satellite images while on a smart phone in the field.

IME it’s impossible to get any authoritative response to questions about the future if addons on Android.

I suggest you also raise an issue at https://github.com/mozilla-mobile/fenix/issues which does at least seem be be read by the Fenix team.

Hi @GavinB, we do not have any plans to support unlisted extensions on Android at this time.

@GavinB You might add your use case to https://github.com/mozilla-mobile/fenix/issues/18135 which is still open.

It can’t? I have an add-on that uses innerHTML() and it’s on the store…

How do it you get it approved?

  • mine uses innerHTML on a site that is not my own domain (google calendar) .

Not only is it approved, but it’s in the Recommended Extensions program.

I tried to make an add-on before that used innerHTML and it was automatically rejected. My current one uses innerHTML in 1 JavaScript file for a third-party library that’s bundled with the add-on. The JavaScript file looks like it’s a generated file, not something that’s manually created by a person. Maybe it being part of a library instead of my actual code has something to do with it?

I still get a warning every time I upload to AMO, but it still passes review. I’m not really sure how much of the review process is human and how much is automated, so maybe at some point a human just allows it?

Honestly, I’m not really sure. To be honest, I’m working to get rid of the innerHTML in my code to clear the warning because I am worried that at some point it will just be rejected.

The "no innerHTML" rule is afaik mostly about being able to reason about what is getting added to the document, thus “Ensure you insert remote content safely” at https://extensionworkshop.com/documentation/develop/build-a-secure-extension/

Hi all,

First, “All add-ons are subject to [the add-on] policies, regardless of how they are distributed.” as stated on the policies page. This includes usage of innerHTML.

innerHTML is generally discouraged but allowed under certain conditions. In any case, any remote or untrusted data assigned to an element via innerHTML needs to be sanitized beforehand. Please refer to our guide.

In your own interest however, you’d want to avoid innerHTML whenever you can. It’s not only easy to open up your add-on to serious security vulnerabilities, innerHTML is also considerably slower than other methods of adding DOM elements. It should never be used out of convenience, like we have seen many times, where a plain text string is assigned.

2 Likes

Ok I’ve now gone ahead and submitted a version to AMO. :crossed_fingers:

i found an other way . Suppose I appended an element with appendChild and creatElement and then you use outerHTML to completely rewrite it so you can add and/or nest more child elements

This has the same security concerns as innerHTML and insertAdjacentHTML. But maybe the code checker isn’t checking for it?

I’m not using innerHTML in my add-on because of all the reasons already laid out above. Instead I’m using this bit (originally part of an object, hence the style):

/**
 * Empty an element whilst avoiding `innerHTML`;
 * @param {HTMLElement} element 
 * @returns 
 */
empty: element => {
    while (element.lastChild) {
        element.lastChild.remove();
    }
    element.textContent = '';
    return element;
}

I got the idea quite possibly from Stack overflow, but I honestly don’t remember where.

sorry I am abit new but always wondered that outer must have same issues as inner . But yeah , the codecheker doesnt show errors/warnings.

There’s a much more efficient/direct way to empty a node these days: https://developer.mozilla.org/en-US/docs/Web/API/Element/replaceChildren#emptying_a_node

1 Like