See testcase, which crashes Mozilla within 200ms after load. It also crashes branch builds, so marking security sensitive for now. Talkback ID: TB32819385Z nsContentUtils::ContentIsDescendantOf [mozilla/content/base/src/nscontentutils.cpp, line 1149] nsCounterList::RecalcAll [mozilla/layout/base/nscountermanager.cpp, line 176] RecalcDirtyLists [mozilla/layout/base/nscountermanager.cpp, line 274] nsBaseHashtable<nsStringHashKey,nsCOMPtr<nsIVariant>,nsIVariant *>::EnumerateRead [mozilla/dist/include/xpcom/nsbasehashtable.h, line 188] nsCounterManager::RecalcAll [mozilla/layout/base/nscountermanager.cpp, line 281] mozAutoDocUpdate::~mozAutoDocUpdate [mozilla/dist/include/content/nsidocument.h, line 986] nsGenericElement::RemoveChildAt [mozilla/content/base/src/nsgenericelement.cpp, line 2628] nsGenericElement::doRemoveChild [mozilla/content/base/src/nsgenericelement.cpp, line 3242] nsGenericElement::RemoveChild [mozilla/content/base/src/nsgenericelement.cpp, line 2812] XPCWrappedNative::CallMethod [mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2229]

Does the treerow element have to be root for this crash, or does it still crash if you wrap it all in a window?

Bug 384728 has a similar stack trace, but involves <svg:use> instead of mathml and trees.

We're failing to remove frames here when removing a content node. I think we probably want this fix on branches too...

Note that I doubt the mathml matters that much, by the way.

Wouldn't it be more robust here to check the container's frame type? Or something like that?

The point is that some of these containers just never have frames... I guess we could check that too.

And I'm a little worried about performance impact of GetPrimaryFrameFor() calls in this code if we do serious population of listboxes.

Given that we'd be getting the same primary frame over and over again, wouldn't that be reasonably fast?

This testcase looks like a null deref in 2.0.0.4 (debug), is this an exploitable situation?

I'm not sure whether we can tweak the testcase to be exploitable. It's possible that this is always a null deref... But I can't guarantee that, given the invariants this ends up violating.

On trunk I see the following assertions with this testcase ###!!! ASSERTION: null check on startContent should be sufficient to null check nodeContent as well, since if nodeContent is for the root, startContent (which is before it) must be too: 'nodeContent || !startContent', file c:/dev/fftrunk/mo zilla/layout/base/nsCounterManager.cpp, line 145 ###!!! ASSERTION: The possible descendant is null!: 'aPossibleDescendant', file c:/dev/fftrunk/mozilla/content/base/src/nsContentUtils.cpp, line 1143

Those assertions also fire in bug 385866.

Boris, I like this better ... what do you think? I think this is more clear about what's going on.

Comment on attachment 270669 [details] [diff] [review] better fix? Sure. That works too. Let's also get this in on the branches.

checked in.

Comment on attachment 270669 [details] [diff] [review] better fix? approved for 1.8.1.5 and 1.8.0.13, a=dveditz

Comment on attachment 270669 [details] [diff] [review] better fix? tree closing early, non-blocker to next release.

Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre

Comment on attachment 270669 [details] [diff] [review] better fix? moving approval to 1.8.0.14 to match 1.8.1 branch landing

Comment on attachment 270669 [details] [diff] [review] better fix? approved for 1.8.1.7, a=dveditz for release-drivers

checked in on branches.

verified fixed 1.8.1.7 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.7pre) Gecko/2007091204 BonEcho/2.0.0.7pre no crash on testcase - adding verified keyword

No crash on the testcase in comment 0 with neither Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre nor Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre Replacing fixed1.8.0.14 keyword with verified1.8.0.14

crash test landed http://hg.mozilla.org/mozilla-central/rev/92419627df02