The machinery to work out which argument types to mark relies on template specialization, but the default case is not to mark. Unfortunately some types are missing so these don't get marked. I guess this is a security issue now we use exact rooting, although I think this would be very hard to exploit in practice.

This adds specializations for handles to JSScript, StaticBlockObject and StaticWithObject, and also a Handle<T> specialization that will cause a build error if other unsupported handles are used.

Comment on attachment 8482289 [details] [diff] [review] add-TypeToRootType-specializations Review of attachment 8482289 [details] [diff] [review]: ----------------------------------------------------------------- Great find!

Does this have impact to ESR?

Jon: security bugs need to follow the sec-approval process before landing https://wiki.mozilla.org/Security/Bug_Approval_Process This is in part to answer the questions bkerensa has above. Please retroactively request sec-approval? on the patch which will populate a template of questions for you to answer about the code being patched.

(In reply to Daniel Veditz [:dveditz] from comment #6) Sorry about that, and thanks for the link - I can never find that page.

Comment on attachment 8482289 [details] [diff] [review] add-TypeToRootType-specializations [Security approval request comment] Requesting retroactive approval for this. How easily could an exploit be constructed based on the patch? I think this would be difficult. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? Exact rooting was enabled on desktop in FF28 and for b2g in 33, so I think the following are affected: aurora, beta, release, esr31, b2g 2.1. If not all supported branches, which bug introduced the flaw? Bug 753203 (but note this was enabled in desktop and b2g builds at different times). Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? No, but should be trivial. How likely is this patch to cause regressions; how much testing does it need? Unlikely. This has been present on central for a month.

Please request aurora/beta/esr31 approval when you get a chance :)

Comment on attachment 8482289 [details] [diff] [review] add-TypeToRootType-specializations After the fact, sec-approval+. We should get this on Aurora but I believe it is too late for ESR31 and Beta. We're under two weeks from release.

We still have time to get this in for the final 33 build if the nominations and approvals are done expediently. Jon, can you please do the aurora/beta/esr31 approval requests ASAP? Thanks :)

Also, this applies without conflict all the way to esr31 and is green on Try.

Comment on attachment 8482289 [details] [diff] [review] add-TypeToRootType-specializations [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Bug marked sec-high User impact if declined: Potential security vulnerability Fix Landed on Version: FF35 Risk to taking this patch (and alternatives if risky): Low String or UUID changes made by this patch: None See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info. Approval Request Comment [Feature/regressing bug #]: Bug 753203 [User impact if declined]: Potential security vulnerability [Describe test coverage new/current, TBPL]: This has been on mozilla-central for a month. [Risks and why]: Low risk [String/UUID change made/needed]: None

https://hg.mozilla.org/releases/mozilla-aurora/rev/77dcdf987c19 https://hg.mozilla.org/releases/mozilla-beta/rev/bbc35ec2c90e https://hg.mozilla.org/releases/mozilla-esr31/rev/be0ae4dbb1ff

Marking [qe-verify-] as I don't see any tests or STR to reproduce. If you come up with something, let me know and I'd be happy to test.