It checks vp->isObject(), but of course strings live in compartments too. This causes tests to fail with the patch for bug 822691. We need to fix this on m-c and aurora; it's a regression from bug 801712.

Comment on attachment 696092 [details] [diff] [review] MaybeWrapValue should work with all gcthings. [Security approval request comment] How easily could an exploit be constructed based on the patch? Probably pretty difficult. I don't know how to do it. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? To some extent. You have to know what gcthings are not objects, and how to trigger this code. Which older supported branches are affected by this flaw? Nothing released; just Aurora 19. If not all supported branches, which bug introduced the flaw? Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? I have an Aurora backport. How likely is this patch to cause regressions; how much testing does it need? I don't think this is likely to cause regressions.

Comment on attachment 696094 [details] [diff] [review] Patch for Aurora [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 801712 User impact if declined: On m-c, crashes. On Aurora, possible security problems. Testing completed (on m-c, etc.): Passes the tests that fail without it. Risk to taking this patch (and alternatives if risky): Very low risk, I believe. String or UUID changes made by this patch: None.

Let's try to get this in for Aurora. Waiting for release management approval here.

Failure to wrap sounds like sec-crit to me, if it can be exploited.

Comment on attachment 696094 [details] [diff] [review] Patch for Aurora Low risk fix for a for a sec-crit FF19 regression.Approving on aurora.