747580 - [security review] iframe sandbox

Status: VERIFIED FIXED

(mozilla.org :: Security Assurance: Review Request, task)

Description

[Ian Melven :imelven]

* Who is/are the point of contact(s) for this review?

[e-mail]

* Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):

the HTML5 iframe sandbox feature adds a sandbox attribute to iframes that can be used to remove their permissions to perform certain actions such as open windows, execute scripts, inherit their natural origin, submit forms and navigate the top level document

* Please provide links to additional information (e.g. feature page, wiki) if available and not yet included in feature description:

https://wiki.mozilla.org/Features/Platform/Iframe_Sandbox

HTML5 spec for iframe sandbox : http://dev.w3.org/html5/spec/the-iframe-element.html#attr-iframe-sandbox

* Does this request block another bug? If so, please indicate the bug number

yes, bug 341604 - this bug contains the code patches for this feature, as well as a few sets of mochitests

* This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?

no urgency - the patches for the code and the tests haven't been reviewed yet

* Please answer the following few questions: (Note: If you are asked to describe anything, 1-2 sentences shall suffice.)
        
  * Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?

yes - Firefox

* Are there any portions of the project that interact with 3rd party services?

no

* Will your application/service collect user data? If so, please describe 

no

* If you feel something is missing here or you would like to provide other kind of feedback, feel free to do so here (no limits on size):

i am particularly looking for feedback on tests to add to the existing ones in the bug to increase test coverage for the feature 

* Desired Date of review (if known from https://mail.mozilla.com/home/[e-mail]/Security%20Review.html) and whom to invite. 

no particular date desired - invite : dveditz and anyone else from security engineering or security assurance who's interested in this feature, it would be great if one of the more web app focused security folk could join too as i'm very interested to hear about potential use cases for this on mozilla's web sites.

Comment 1

[Ian Melven :imelven]

Please see the tests in bug 341604 - i am looking for cases that are not being tested. In his first review of bug 341604 smaug suggested tests around navigation and the bfcache, so i am planning on adding some of those already.

Comment 2

[Ian Melven :imelven]

I'm going to speak to dveditz about this again tomorrow to see what else we might need to do to land bug 341604.

Comment 3

[Ian Melven :imelven]

Should we mark this RESOLVED FIXED NOW ? 

There's more work on iframe sandbox but it's broken out into followup bugs - I will ask for a security review on the followup bugs that need one like 'allow-popups', for instance.

Comment 4

[Ian Melven :imelven]

For the record, when I spoke to dveditz, he was fine with this landing, we couldn't think of any more test cases that aren't already covered in the tests that are landing as part of bug 341604.

Comment 5

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

I don't think we have any issues with this landing, however, lets create dependent bugs to this bug for follow-up bugs just for ease of tracking please. We can still mark this as resolved-fixed and leave confirmed fixed until the follow-up bugs are done then. Sound good?