"Unsafe assignment to innerHTML" with LinkeDOM

regseb · December 6, 2022, 2:05pm

Hi,

I am currently working on upgrading my extension Cast Kodi to MV3. The background must parse HTML, but DOMParser is not available in the service worker. So I use LinkeDOM as polyfill. Addons-linter reports three warnings : Unsafe assignment to innerHTML.

Do you think my extension will be accepted in addons.mozilla.org?

The LinkeDOM library is only useful for Chromium, but I don’t want to manage two versions of my code (or develop a process to generate two versions).

juhis · December 9, 2022, 7:20pm

Does the innerHTML assignment happen inside LinkeDOM or in your code?

regseb · December 9, 2022, 8:17pm

The calls to innerHTML are in LinkeDOM.

github.com

WebReflection/linkedom/blob/c3a207b/worker.js#L7393-L7398


get outerHTML() { return this.toString(); }
set outerHTML(html) {
  const template = this.ownerDocument.createElement('');
  template.innerHTML = html;
  this.replaceWith(...template.childNodes);
}

github.com

WebReflection/linkedom/blob/c3a207b/worker.js#L7572-L7576


insertAdjacentHTML(position, html) {
  const template = this.ownerDocument.createElement('template');
  template.innerHTML = html;
  this.insertAdjacentElement(position, template.content);
}

github.com

WebReflection/linkedom/blob/c3a207b/worker.js#L11441-L11446


createContextualFragment(html) {
  const template = this.commonAncestorContainer.createElement('template');
  template.innerHTML = html;
  this.selectNode(template.content);
  return template.content;
}

I think LinkeDOM doesn’t interpret JavaScript. So there should be no risk to use this library.

juhis · December 9, 2022, 7:59pm

I think the browser still interprets it, even if it comes through a library like LinkeDOM.

In general, the worry is code that adds unsanitized/unescaped inputs into .innerHTML as that opens it up for security risks.

If the content (eventually being the html parameter in those functions) comes from your extension, you could clean it up there (with for example DOMPurity).

I’ll also chat with the team next week to see if this is a use case our reviewers have experience with and can provide more details on what could be a good solution.

regseb · December 9, 2022, 8:10pm

The goal of LinkeDOM is to provide an implementation of the DOM. The library doesn’t use the innerHTML method of the browser. It has its own implementation:

github.com

WebReflection/linkedom/blob/c3a207b/worker.js#L6852-L6860


const setInnerHtml = (node, html) => {
  const {ownerDocument} = node;
  const {constructor} = ownerDocument;
  const document = new constructor;
  document[CUSTOM_ELEMENTS] = ownerDocument[CUSTOM_ELEMENTS];
  const {childNodes} = parseFromString(document, ignoreCase(node), html);

  node.replaceChildren(...childNodes.map(setOwnerDocument, ownerDocument));
};