I've created my first open source pull request ever, but it couldn't be accepted because the firefox extension I was referencing hadn't its last version approved.

Firefox version not approved #49

paglias posted on Sep 12, 2019

The Firefox version of the extension had been rejected and is not available in the marketplace due to the following problems

1. 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

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

3. Please add a privacy policy to this add-on that details which user data is being sent and to what services. The critical things to describe in the policy are how your extension collects, uses, stores, and shares or discloses information about people.If your add-on makes it apparent to websites that it is installed, this must also be mentioned. The policy should be about the extension only, not a copy of the website's privacy policy. It should also be the actual text, as opposed to a link to a privacy policy on a website. The privacy policy can be added in the add-on settings under “Manage Authors & License” on AMO. Here are some examples that were discovered: mainChat\chat_inPage.js line 55, 182, 276, 569

• [ ] ^ this one will have to be handled by the Habitica staff

4. 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.

View on GitHub

Since I'll be literally working with Javascript soon, this seems a great opportunity to practice the language.

🎑 Environment

Ok, so the first challenge is to understand how to create/maintain a Firefox extension. Of course Mozilla would have a nice documentation about it

😺 Mozilla - Your first WebExtension

It seems that manifest.json is the extension's heart.
And to make it live, we have to go to about:debugging in Firefox, click "Load Temporary Add-on" and select our manifest.json.
Beware to not select the zip file as this will be our deployed version later.

Then we visit Habitica.com and check it out. It's working!

🔧 Fixing issues

📏 Minified Code

Firefox complains the following:

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

It seems that minified code is not allowed, therefore we just have to convert this habitica-markdown.min.js to a not-minified version.

I've simply pasted the code in this UglifyJS and selected Beautify. Then I've created a new habitica-markdown.js and pasted the new beautified code in it.

Sure we have to fix some imports, such as in manifest.json and chat.js

// chat.js
// Call markdown to html script
var s = document.createElement("script");
s.src = browser.extension.getURL("resources/habitica-markdown.js");
s.onload = function() {
  this.parentNode.removeChild(this);
};
(document.head || document.documentElement).appendChild(s);

🛂 Ajax Permission

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

This is as simples as removing the string https://ajax.googleapis.com/ from manifest's permissions key.

It didn't break anything, so it was indeed unused.

🚿 Sanitize HTML Strings

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 is where I most had to search about. Which was great because it was something I was very curious since I had contact with Javascript and Security.

As some might already know, creating DOM nodes using HTML strings (such as innerHTML, jQuery.html) with user input data is very dangerous because it can suffer XSS (Cross Site Scripting) attacks.
To verify this with my own hands, I've taken a line of code from the extension and tried to insert some malicious code in it.

In the code above, I've also converted the HTML String to DOM node methods to confirm if they would indeed be safer.

We could convert all unsafe lines to DOM methods, but there's another way to solve this issue that's with HTML sanitization.

We add purify.js from DOMPurify in our resources files, reference it in manifest.json, import it in chat.js and use it with the following example:

var elem = document.createElement("div");
var cleanHTML = DOMPurify.sanitize(externalHTML);
elem.innerHTML = cleanHTML;

(when using with JQuery, the flag { SAFE_FOR_JQUERY: true } is required)

In the chat_InPage.js I had to use DOMPurify 24 times.
I didn't find an easier way to do this than manually, so if you know other solutions, feel free to share in the comments.

📧 Submitting the PR

As instructed in the README.md, we need to replicate the changes to Chrome port and test them. Luckily everything worked just fine.
We also need to bump the version in manifest.json and write down the release notes in the README itself.
We can't forget to zip files (not folder) with all the changes we've made to package the extension.

Fix firefox extension #50

Markkop posted on Oct 02, 2019

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

View on GitHub

🔨 Conclusion

Working on this issue was pretty interesting, because it involved Javascript, Browser Extensions and Security. I've even had the opportunity to learn more about XSS attacks and how to prevent them.
Now I have to wait for a review and for Habitica's staff to handle the privacy policy which is another requirement for Firefox.