View source for AMO:Editors/EditorGuide/AddonReviews/Security

== Security Code Review ==
The table on the [[AMO:Editors/EditorGuide/AddonReviews#Policies_and_Actions_3|Review Guide]] gives a summary of issues we check for.  The sections below replicate these rejection reasons but with more detail and examples.

(This page is still in progress; some sections are empty and others may contain incorrect examples at the moment!)

=== Using eval, Function(), setTimeout, setInterval to evaluate remote code ===
Using eval() at all in an add-on is generally prohibited, which some exceptions (notably patching Firefox functions where there are no other alternatives).  

eval("code;") is effectively the same as setTimeout("code;",1) or setInterval("code;",1);
==== Examples of common (mis)-uses of eval and similar functions ====
<pre>
var output = eval(r.responseText);  //mistakenly used to parse JSON - any code will be executed.

var data=r.responseText;
setTimeout("doSomething('"+data+"');",100);  //would be executed if data contained a ' character.
</pre>
In the case of JSON parsing there is a canned response referring developers to https://developer.mozilla.org/en/Using_JSON_in_Firefox

=== <browser> or <iframe> elements with no type ===
If a <browser> or <iframe> tag is used in a xul document then by default it loads any content inside with the same privileges as the xul document containing it.  So a <iframe> tag in an overlay will load pages with full chrome access by default.  

A tag with type="content", type="content-targetable" or type="content-primary" will safely load any page within as the user would normally load a webpage.

There is a canned response that points developers to this page: https://developer.mozilla.org/En/XUL/Iframe#a-browser.type
==== Bad (and Good) examples ====
Bad - no type parameter:
<pre><iframe src="https://www.google.com/"></pre>

Good - type set to content:
<pre><iframe src="https://www.google.com/" type="content"></pre>

<b>The type attribute <em>must</em> be set before any page is loaded or it is ignored.</b>  (There is a [https://bugzilla.mozilla.org/show_bug.cgi?id=705271 request] to stop this)

Bad - src set before type:

<i>In overlay.xul</i>
<pre><iframe id="ifr"></pre>
<i>In overlay.js</i>
<pre>
var ifr=getElementById("ifr");
ifr.setAttribute("src","http://www.google.com"); //google now has control of Firefox!
ifr.setAttribute("type","content"); //ignored because src set on the line above
</pre>

Good - src set after type:

<i>In overlay.xul</i>
<pre><iframe id="ifr"></pre>
<i>In overlay.js</i>
<pre>
var ifr=getElementById("ifr");
ifr.setAttribute("type","content"); 
ifr.setAttribute("src","http://www.google.com");
</pre>

Looks Good but Bad also (about:blank loaded so type setting ignored):

<i>In overlay.xul</i>
<pre><iframe src="about:blank" id="ifr"></pre>
<i>In overlay.js</i>
<pre>
var ifr=getElementById("ifr");
ifr.setAttribute("type","content"); //ignored because about:blank was loaded when overlay.xul was loaded
ifr.setAttribute("src","http://www.google.com"); //google now has control of Firefox!
</pre>

=== Remote script injection  ===
We generally don't allow addons to insert remote JavaScript into web pages as we can't guarantee the content will stay the same as when we review it.  HTTP insertion is also vulnerable to MITM attacks (where the content is tampered with in transit) and will break the seal on HTTPS pages.

==== Examples of remote script insertion ====
Literal tag in xul/html:
<pre>
<script src="http://www.google.com/script.js />
</pre>
Insertion of tag from a script into a document:
<pre>
var s=document.createElement("script");
s.setAttribute("src","http://www.google.com/script.js");
document.appendChild(s);
</pre>
Retrieving remote content and then setting the tag to the content
<pre>
var data=r.responseText;
var s=document.createElement("script");
s.innerHTML=data;
document.appendChild(s);
</pre>

=== Inserting remote content with innerHTML ===
...

=== Remote code download or execution ===
...

=== Custom update code ===
...

=== HTTP security violations ===
...