# Sanitizing data injected into the DOM

**URL:** <https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028>\
**Category:** Development\
**Created:** [November 15, 2016, 2:46am UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028 "2016-11-15T02:46:25Z")\
**Posts on this page:** 12\
**Page:** 1

<div class="post-metadata">

**Author:** ![discogs.enhancer](https://avatars.discourse-cdn.com/v4/letter/d/a587f6/32.png) [@discogs.enhancer](https://discourse.mozilla.org/u/discogs.enhancer)\
**Post date:** [November 15, 2016, 2:46am UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/1 "2016-11-15T02:46:26Z")

</div>

Hi,

I am attempting to port my Chrome extension over to Firefox but it has been rejected because the data I am injecting into the DOM is not sanitized. The reviewer recommended an [article](https://developer.mozilla.org/en-US/Add-ons/Overlay_Extensions/XUL_School/DOM_Building_and_HTML_Insertion) however, the article says it is deprecated and should not be followed when writing a WebExtension.

I replied asking for a reference that was not deprecated and was told “since you are already using jQuery, you could easily build jQuery objects using JSON notation and append those”.

Is anyone able to explain what the reviewer means by “jQuery objects” or provide some documentation on the topic?

Thanks!

---

<div class="post-metadata">

**Author:** ![erosman](https://sea1.discourse-cdn.com/flex001/user_avatar/discourse.mozilla.org/erosman/32/15447_2.png) [@erosman](https://discourse.mozilla.org/u/erosman)\
**Post date:** [November 15, 2016, 5:11am UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/2 "2016-11-15T05:11:54Z")

</div>

Data can be injected into DOM as data ie as text (with some reservations).

Remote data can NOT be converted to DOM and injected as it may can be a security risk.  
Remote data can NOT be injected as executable script eg as JavaScript or a string that can be executed like href, src, on\*\*\* etc

Inserting text data as text in a non-executable form is fine.

Example of Remote data:

```
var obj = {
  "employees":[
      {"firstName":"John", "lastName":"Doe"},
      {"firstName":"Anna", "lastName":"Smith"},
      {"firstName":"Peter", "lastName":"Jones"}
  ]
}

var p = document.createElement('p');
p.textContent = 'Helllo ' + obj.employees[0].firstName + ' ' + 
                            obj.employees[0].lastName;

document.body.appendChild(p);
```

---

<div class="post-metadata">

**Author:** ![discogs.enhancer](https://avatars.discourse-cdn.com/v4/letter/d/a587f6/32.png) [@discogs.enhancer](https://discourse.mozilla.org/u/discogs.enhancer)\
**Post date:** [November 15, 2016, 3:01pm UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/3 "2016-11-15T15:01:46Z")

</div>

Thanks very much for your reply.

My extension makes ajax requests to one remote domain - [fixer.io](http://fixer.io) - which is a JSON API for foreign exchange rates and currency conversion. It returns JSON, as you might expect, which I then use to allow the user to perform currency conversions with an on-screen converter. Does this data need to be sanitized? It is already in JSON format much like your obj example.

All other ajax requests made by my extension go to the same domain that the extension is designed for - [discogs.com](http://discogs.com) (a massive database/marketplace of music releases).

For example, one feature the extension offers is a feedback notifier for when another user leaves buyer/seller feedback on your profile. The extension periodically polls your feedback page, extracts data from the returned markup and stores that in an object in localStorage. It performs a comparison of the current data with the data stored in localStorage. If the data has changed since the last poll it inserts a notification badge into the header of the site which the user can click and be taken to the proper feedback page.

Does this need to be sanitized? It is coming from the same domain and is basically screen-scraped data. This seems safe to me but I am by no means a security expert.

---

<div class="post-metadata">

**Author:** ![erosman](https://sea1.discourse-cdn.com/flex001/user_avatar/discourse.mozilla.org/erosman/32/15447_2.png) [@erosman](https://discourse.mozilla.org/u/erosman)\
**Post date:** [November 15, 2016, 3:37pm UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/4 "2016-11-15T15:37:42Z")

</div>

JSON data has to be made safe. It is not that straightforward to sanitize the JSON data.Therefore the best action is to inject the data in a format that would be safe.

Using eval(), innerHTML (or similar eg JQuery append) etc on remote content is a security risk and thus cause for rejection.

Which addon is it? If it is on AMO, I will try to have a quick look at the code.

---

<div class="post-metadata">

**Author:** ![discogs.enhancer](https://avatars.discourse-cdn.com/v4/letter/d/a587f6/32.png) [@discogs.enhancer](https://discourse.mozilla.org/u/discogs.enhancer)\
**Post date:** [November 15, 2016, 6:20pm UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/5 "2016-11-15T18:20:18Z")

</div>

Thanks again for your feedback!

Here is the link to the disabled AMO listing: [https://addons.mozilla.org/en-US/developers/addon/discogs-enhancer](https://addons.mozilla.org/en-US/developers/addon/discogs-enhancer)

And here is the link to the source on my GitHub profile: [https://github.com/salcido/Discogs-Enhancer-Firefox](https://github.com/salcido/Discogs-Enhancer-Firefox)

---

<div class="post-metadata">

**Author:** ![erosman](https://sea1.discourse-cdn.com/flex001/user_avatar/discourse.mozilla.org/erosman/32/15447_2.png) [@erosman](https://discourse.mozilla.org/u/erosman)\
**Post date:** [November 15, 2016, 6:53pm UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/6 "2016-11-15T18:53:52Z")

</div>

There are too many files to check for a quick check. Give me an example of AJAX JSON insertion and I will look at it for you.

---

<div class="post-metadata">

**Author:** ![discogs.enhancer](https://avatars.discourse-cdn.com/v4/letter/d/a587f6/32.png) [@discogs.enhancer](https://discourse.mozilla.org/u/discogs.enhancer)\
**Post date:** [November 15, 2016, 7:26pm UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/7 "2016-11-15T19:26:11Z")

</div>

I’m sorry about that. I should have been more specific.

Here is my code for the currency converter as an example:

[https://github.com/salcido/Discogs-Enhancer-Firefox/blob/master/js/currency-converter.js](https://github.com/salcido/Discogs-Enhancer-Firefox/blob/master/js/currency-converter.js)

There is a `GET` request on line `142` to `api.fixer.io`.

JSON (representing various currencies) is returned and then stored in `localStorage` for later reference.

The markup for the converter itself is a `string` defined on line `21` which is later injected into the DOM with jQuery’s `append` method on line `208`.

The converter will take the value supplied by the user, convert it to the specified currency using the JSON data that was previously saved to `localStorage`, and then set the value of the `span` with the newly converted value.

---

<div class="post-metadata">

**Author:** ![erosman](https://sea1.discourse-cdn.com/flex001/user_avatar/discourse.mozilla.org/erosman/32/15447_2.png) [@erosman](https://discourse.mozilla.org/u/erosman)\
**Post date:** [November 16, 2016, 4:59am UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/8 "2016-11-16T04:59:38Z")

</div>

The JSON response if fine on its own but at the time of insertion it has to be made safe.

`markup` is a sting which is then converted to DOM via `$('body').append(markup);`  
While above is not a security risk as it is, it is bad practice and has performance implications.  
In general, converting strings to DOM is not a good idea. Additionally, whenever strings are converted to DOM (through whatever method) it has to be checked thoroughly to make sure they are safe.

As you are converting values to integers, the result is safe eg:  
`result = (input.val() * rates.rates[thatSelectedCurrency]).toFixed(2);`

If you change the process, it should be safe.  
Avoid converting strings to DOM ie `markup` or the following etc:  
`output.html('&#175;\\\_(&#12484;)_/&#175;');`

`JQuery.html()` is similar to `innerHTML` and converts strings to DOM. While above is safe, the better way is to use `textContent` or `JQuery.text()`.

Instead of the HTML entities, you can use their Unicode which works fine with `textContent`.

Finally, another problem of converting strings to DOM is that reviewer has to track each variable through many files to find out where they have come from and if they are safe. That significantly complicates the review process and adds to your waiting time.

---

<div class="post-metadata">

**Author:** ![discogs.enhancer](https://avatars.discourse-cdn.com/v4/letter/d/a587f6/32.png) [@discogs.enhancer](https://discourse.mozilla.org/u/discogs.enhancer)\
**Post date:** [November 16, 2016, 11:43pm UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/9 "2016-11-16T23:43:39Z")

</div>

Thank you for taking the time to read through my code. Based on your comments it sounds like while I am using an approach that is frowned upon, the code is not unsafe.

> While above is not a security risk as it is, it is bad practice and has performance implications. In general, converting strings to DOM is not a good idea.

I understand that it is less performant but the trade off for me is that it is significantly easier to maintain. The markup that is needed is relatively complex and it’s much easier to maintain a string that reads like `html` than it is to write a pure Javascript representation of each node and their attributes and then inject them accordingly.

By doing things this way, will my extension ever pass review? Because of the complexity of my extension I get the impression that the reviewer does not want to take the time to actually read through my code.

---

<div class="post-metadata">

**Author:** ![erosman](https://sea1.discourse-cdn.com/flex001/user_avatar/discourse.mozilla.org/erosman/32/15447_2.png) [@erosman](https://discourse.mozilla.org/u/erosman)\
**Post date:** [November 17, 2016, 6:02am UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/10 "2016-11-17T06:02:47Z")

</div>

> [@discogs.enhancer](#):
>
> Based on your comments it sounds like while I am using an approach that is frowned upon, the code is not unsafe.

I only looked at one instance of remote data injection. I have not checked the rest of the files/code.

---

<div class="post-metadata">

**Author:** ![discogs.enhancer](https://avatars.discourse-cdn.com/v4/letter/d/a587f6/32.png) [@discogs.enhancer](https://discourse.mozilla.org/u/discogs.enhancer)\
**Post date:** [November 17, 2016, 7:53pm UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/11 "2016-11-17T19:53:41Z")

</div>

This is the only call to an external domain that my extension uses. All the other `GET` requests are made to the site the extension is designed to work with.

---

<div class="post-metadata">

**Author:** ![NilkasG](https://avatars.discourse-cdn.com/v4/letter/n/0ea827/32.png) [@NilkasG](https://discourse.mozilla.org/u/NilkasG)\
**Post date:** [November 17, 2016, 8:08pm UTC](https://discourse.mozilla.org/t/sanitizing-data-injected-into-the-dom/12028/12 "2016-11-17T20:08:18Z")

</div>

> [@discogs.enhancer](#):
>
> All the other GET requests are made to the site the extension is designed to work with.

Form the perspective of Firefox/amo, **all** web requests are external (except to `localhost`, maybe).

When you submit an add-on the reviewers can only verify that the code you are submitting is ok.  
They cant say anything about the code you may be serving from you website, and especially can’t know how that will change in the future.  
If you are using strings that are not sanatized incorrectly, they are potentially code, and that code is untrusted no matter which domain it comes from.