AMO:Editors/EditorGuide/AddonReviews/Security
Contents
- 1 Security Code Review
Security Code Review
The table on the 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
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.
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:
<iframe src="https://www.google.com/">
Good - type set to content:
<iframe src="https://www.google.com/" type="content">
The type attribute must be set before any page is loaded or it is ignored. (There is a request to stop this)
Bad - src set before type:
In overlay.xul
<iframe id="ifr">
In overlay.js
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
Good - src set after type:
In overlay.xul
<iframe id="ifr">
In overlay.js
var ifr=getElementById("ifr"); ifr.setAttribute("type","content"); ifr.setAttribute("src","http://www.google.com");
Looks Good but Bad also (about:blank loaded so type setting ignored):
In overlay.xul
<iframe src="about:blank" id="ifr">
In overlay.js
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!
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:
<script src="http://www.google.com/script.js />
Insertion of tag from a script into a document:
var s=document.createElement("script"); s.setAttribute("src","http://www.google.com/script.js"); document.appendChild(s);
Retrieving remote content and then setting the tag to the content
var data=r.responseText; var s=document.createElement("script"); s.innerHTML=data; document.appendChild(s);
Inserting remote content with innerHTML
...
Remote code download or execution
...
Custom update code
...
HTTP security violations
...