HabitRPG / habitica-chat-extension

A habitica.com Chat Client for Chrome
18 stars 13 forks source link

Fix firefox extension #50

Closed Markkop closed 4 years ago

Markkop commented 4 years ago

Fixes #49 (partially)

This version contains minified, concatenated or otherwise machine-generated code. Please provide the original sources, together with instructions on how to generate the final XPI. Source code must be provided as an archive and uploaded using the source code upload field, which can be done during submission or on the version page in the developer hub. Please read through the instructions at https://developer.mozilla.org/en-US/Add-ons/Source_Code_Submission. Here are some examples that were discovered: resources\habitica-markdown.min.js

I've used this tool to unminify habitica-markdown.min.js I had to update manifest.json and chat.json with the new habitica-markdown.js file

Please remove all unused permissions from your manifest. Here are some examples that were discovered: https://ajax.googleapis.com/

I've removed this permission from manifest.json

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: mainChat\chat_inPage.js line 147, 194, 199 and possible more.

This was more troublesome. I believe it has 2 solutions: convert HTML strings to proper html elements being created with Javascript or using a DOMPurifier as recommend by Firefox This lib is imported at chat.js and applied in several parts of chat_inPage.js

All changes were ported to Chrome's version and tested.

The privacy policy is still needed and have to be handled by Habitica Staff.

PS: some formatting have been changed, sorry about that PS2: this fix has been documented in this article

paglias commented 4 years ago

Thanks @Markkop !

For the habitica-markdown source code I think the best thing would be to directly use the content of this file https://github.com/HabitRPG/habitica-markdown/blob/master/dist/habitica-markdown.js since it's the original source and the un-minified version still has some weird variable names

For the dompurify good find! I'd add a comment at the top of source code describing the license it's released under to avoid any type of problems, see https://github.com/cure53/DOMPurify/blob/master/LICENSE I think this part will be enough

DOMPurify
Copyright 2015 Mario Heiderich

DOMPurify is free software; you can redistribute it and/or modify it under the
terms of either:

a) the Apache License Version 2.0, or
b) the Mozilla Public License Version 2.0

After these changes are made I'll try to resubmit it with an updated privacy policy, hopefully it's not rejected again, otherwise we'll have to start using a proper build system to avoid using the source code of external dependencies

Markkop commented 4 years ago

Nice catch about using the original habitica-markdown source code, it makes a lot of sense!

paglias commented 4 years ago

thanks @Markkop ! Sorry for the delay, if you have an Habitica account please let me know your UUID or username and I'll award a contribution tier

Markkop commented 4 years ago

thanks @Markkop ! Sorry for the delay, if you have an Habitica account please let me know your UUID or username and I'll award a contribution tier

No problem, thanks for reviewing! Here's my UUID: 40387571-91ee-489e-960f-278bf8fd503a :rocket:

paglias commented 4 years ago

The extension is under review, hopefully it'll be approved this time. Notated towards your 4th tier and added the Blacksmith title