fregante / notifications-preview-github

Browser Extension: preview GitHub notifications with same page pop-overs
https://chrome.google.com/webstore/detail/github-notifications-prev/kgilejfahkjidpaclkepbdoeioeohfmj?hl=en&gl=IN
MIT License
144 stars 15 forks source link

Publish on AMO for Firefox users #26

Closed jdreesen closed 7 years ago

jdreesen commented 7 years ago

This extension looks awesome and I'd really like to use it in my Firefox browser.

Sadly, I've got zero experience in developing Firefox addons so I doubt it can help much here. But I'll try to look a little bit into it tomorrow and try to install it in my Firefox in dev mode to see if there are any incompatibilities or if it runs fine.

fregante commented 7 years ago

It's been published 🎉, now we're waiting for its approval on AMO:

https://addons.mozilla.org/en-US/firefox/addon/notifications-preview-github/

jdreesen commented 7 years ago

Woah, you are awesome! Thanks a lot!

Sorry I didn't found the time to look into it in the last days :/

fregante commented 7 years ago

Just to be clear, this is the kind of response we get from AMO reviewers:

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/XUL_School/DOM_Building_and_HTML_Insertion . Here are some examples that were discovered:

index.js#L108 rawNotifications = fetch(location.origin + '/notifications', { index.js#L78 const notificationsPage = await rawNotifications.then(domify);

index.js#L22 function domify(html) { return new DOMParser().parseFromString(html, "text/html"); }

index.js#L91 const container = select('#NPG-dropdown'); empty(container); container.append(...notificationsList);

index.js#L59 indicator.parentNode.insertAdjacentHTML('beforeend', ........ <div id="NPG-dropdown"...

Addon is getting content from a remote location, converts them to DOM and inserts them into the page as DOM which is a security issue. If remote data was inserted as text, it could have been acceptable.

Please fix them and submit again. Thank you.

Please note the following for the next update: 1) Please make your add-on listing more elaborate. Your summary should briefly explain what the add-on does, so that even people without knowledge about your add-on and its services can understand. The description should give details on the add-on features, how to configure and if not apparent then how to use it. The listing should also contain one or more screenshots.

fregante commented 7 years ago

Guess what! Issues solved! I got on the IRC channel and figured out what they needed from us. It's live on AMO! https://addons.mozilla.org/en-US/firefox/addon/notifications-preview-github/