Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vimium has been disabled on the Mozilla addons site because of innerHTML usage #4195

Closed
philc opened this issue Dec 20, 2022 · 10 comments
Closed

Comments

@philc
Copy link
Owner

philc commented Dec 20, 2022

Upon submitting 1.67.5, the Firefox review team disabled all versions of Vimium since 4 years ago (!) for the reason below. This looks like a bogus reason, since the content being injected into options.js is HTML packaged with the extension.

This add-on didn't pass review because of the following problems:

  1. This add-on is creating DOM nodes from HTML strings containing potentially unsanitized data, by assigning to innerHTML, jQuery.html, or through similar means. Aside from being inefficient, this is a major security risk. For more information, see https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Safely_inserting_external_content_into_a_page . Here are some examples that were discovered: pages\options.js - line 429

Version(s) affected and disabled:
1.65, 1.65.1, 1.66, 1.67, 1.67.1, 1.67.2, 1.67.3, 1.67.4, 1.67.5

@spiderbit
Copy link

Is there a updated version or xpi file for 1.67.5 or something I am stuck at 1.67.4 which has a broken yank feature in firefox.

@philc
Copy link
Owner Author

philc commented Dec 21, 2022

Reopening this for awareness because they continue to reject Vimium for spurious reasons; I'm having back and forth with the approval team.

@philc philc reopened this Dec 21, 2022
@citosid
Copy link

citosid commented Dec 21, 2022

@philc do you have a list of all the changes they want you to do? Maybe we could help changing it the way they want (maybe just doing this is enough?)

Although I agree is weird this extension being alive and being used for so many years and only now being flagged...

Thanks for all your work!

@gdh1995
Copy link
Contributor

gdh1995 commented Dec 25, 2022

Um, according to my memory, Firefox Add-Ons requires an extension doesn't use dynamic value for ".innerHTML", which means .innerHTML = "" is OKay while .innerHTML = [...].join("") isn't.

Added: the officials require there's no dynamic content for .innerHTML in all JS code of an extension. So it's not enough to just update pages/options.js.

A suggested way is to wrap dynamic HTML string with DOMParser:

var html = "<div>...</div>";
var divElement = new DOMParser().parseFromString(html, "text/html").body.firstElementChild
// insert `divElement` or its children if needed.

@philc
Copy link
Owner Author

philc commented Jan 2, 2023

Still waiting for them to respond to me...

@WhiteBlackGoose
Copy link

You're doing an amazing job, thank you so much!

@xarthna
Copy link

xarthna commented Jan 3, 2023

Thank you @philc, looking forward to seeing this resolved.

@RenanSFreitas
Copy link

Thank you very much, @philc 🙂 🤞

@aalvarado
Copy link

My version shows up correctly now and it requested clipboard permissions. I think this can be marked as resolved.

Many thanks @philc @smblott-github

@philc
Copy link
Owner Author

philc commented Jan 6, 2023

The Firefox team reversed their rejection decision without any further comment or notification to me, and v1.67.6 is now on the Firefox addons site. Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants