###!!! ASSERTION: Unexpected aDocument: 'aDocument == mDocument', file layout/base/nsPresShell.cpp, line 4073 ###!!! ASSERTION: Should be in an update while creating frames: 'mUpdateCount != 0', file layout/base/nsCSSFrameConstructor.cpp, line 6532 And a memory leak. (Security-sensitive because the second assertion has appeared in security bugs.)

Jesse, is this a regression from bug 821850? Does it depend on the pref being flipped?

This happens with a clean profile / default prefs. I don't know whether it's a regression.

Ok, I think this is unrelated to my XBL stuff. Though it sounds somewhat like what blake was looking at last week. Blake, is this related?

David, we're unsure how to sec- rate this one. How gloomy is the nsCSSFrameConstructor.cpp assert?

Both assertions sound like things have gone pretty wrong in a way that I could imagine being exploitable, but I can't really make a useful guess about the likelihood/danger off the top of my head. I think it's one of those things where we'll know more once we figure out the fix.

OK, guessing sec-high to split the difference between critical and moderate.

Blake, any thoughts on comment 4?

It's unlikely that this is directly related to what I was looking at, but I can definitely investigate here.

The root of an anonymous subtree is special in that it is not in its parent's child list. Moving such a root out from under its shadow parent confuses the XBL code (and possibly also the BindToTree code). This patch fixes adoptNode to follow nsINode::ReplaceOrInsertBefore's behavior by making sure we don't move the root of an anonymous subtree out from under it.

Comment on attachment 728488 [details] [diff] [review] Proposed fix r=me

Comment on attachment 728488 [details] [diff] [review] Proposed fix [Security approval request comment] How easily could an exploit be constructed based on the patch? The patch points directly at the problem (calling adopt node on the root of an anonymous subtree causes problems). However, I don't think that anybody actually knows how to exploit this. We know that it's dangerous, so we're treating it as sec-high, but coming up with an actual exploit is likely many hours worth of work. Which older supported branches are affected by this flaw? Some digging around showed that bug 650493 is at fault here (it removed an implicit check for the adopted node being in the child list of the parent). How likely is this patch to cause regressions; how much testing does it need? This is extremely unlikely to cause regressions. It introduces an error check that bails out only in cases that would cause this bug.

Comment on attachment 728488 [details] [diff] [review] Proposed fix sec-approval+ but only for a checkin 4/9 or after.

This makes life a little clearer for the future with an additional assertion (r=sicking if anybody's tracking it).

Late in the game to get this into Firefox 21, let's just wait for 22.

Blake - is this ready for uplift to FF22?

Comment on attachment 736049 [details] [diff] [review] For checkin Yes.

Can we get the esr patch as well ?

Sorry, the previous esr17 patch didn't compile.

Verified as fixed on the 05/29 beta debug build on Mac OSX 10.7.5. No assertions are displayed with the attached test case anymore and the test case is displayed fine.

Confirmed assertion 17.0.6esr. Confirmed fixed 17esr 2013-06-18. Confirmed fixed FF23 2013-06-14.

This doesn't seem to have made it to b2g18, although that's marked "affected".

https://hg.mozilla.org/releases/mozilla-b2g18/rev/9d624187768c a=bajaj over email. Did this test ever land? Looks like it was supposed to back in August.

...and that was the wrong testcase, oops.

fixed on central https://hg.mozilla.org/mozilla-central/rev/a301aa4f3e43