Created attachment 267142 [details] testcase 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.
Created attachment 268727 [details] [diff] [review] Proposed fix 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 22.214.171.124 (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.
Created attachment 270669 [details] [diff] [review] better fix? 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.
10 years ago
10 years ago
10 years ago
Comment on attachment 270669 [details] [diff] [review] better fix? approved for 126.96.36.199 and 188.8.131.52, 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 184.108.40.206 to match 1.8.1 branch landing
Comment on attachment 270669 [details] [diff] [review] better fix? approved for 220.127.116.11, a=dveditz for release-drivers
checked in on branches.
verified fixed 18.104.22.168 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:22.214.171.124pre) Gecko/2007091204 BonEcho/126.96.36.199pre 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:188.8.131.52pre) Gecko/20071210 Firefox/184.108.40.206pre nor Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:220.127.116.11pre) Gecko/20071210 Firefox/18.104.22.168pre Replacing fixed22.214.171.124 keyword with verified126.96.36.199
crash test landed http://hg.mozilla.org/mozilla-central/rev/92419627df02