307992 - Crash [@ nsContainerFrame::PositionFrameView] using text zoom in evil document

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050910
Firefox/1.6a1

Please keep this security-sensitive until a Firefox both bug 306663 and bug
306939 have become public.

Comment 1

[Jesse Ruderman]

Attachment: evil testcase

Using text zoom after it finishes loading crashes about 50% of the time.

Comment 2

[Jesse Ruderman]

Attachment: apple crash report with stack trace

Comment 3

[Jesse Ruderman]

I managed to make it crash with a similar stack but a nonzero, random-looking
address at the top of the stack (using parameters other than the one hardcoded
in the testcase).  As a result, I think this crash is exploitable.

Comment 4

[Jesse Ruderman]

Crashes on Windows too.  TB9235998K

Comment 5

[Jesse Ruderman]

Attachment: Windows stack trace from Talkback

Comment 6

[Asa Dotzler [:asa]]

David, can you dig into this one?

Comment 7

[Asa Dotzler [:asa]]

If you come up with a very safe fix in the next couple of days, please request
approval for the patch and we'll evaluate.

Comment 8

[Martijn Wargers (dead)]

Attachment: testcase2, not minimised

While trying to minimise, I quickly got something that crashes directly when clicking at the button (although I takes 10 seconds or so).
Talkback ID: TB11047629Y

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So this actually turned into a DoDeletingFrameSubtree crash for me...

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch that fixes tons of reflow assertions (but not the crash) (checked in to trunk, 2005-12-15) (BELONGS ON BUG 320502)

So this fixes tons of assertions on this testcase, but not the crash (although it might have made it take longer to crash; sicking thinks so, anyway).

This is also something that could potentially fix crashes or potentially fix other StirDOM bugs...

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 206017 [details] [diff] [review]
patch that fixes tons of reflow assertions (but not the crash) (checked in to trunk, 2005-12-15) (BELONGS ON BUG 320502)

(We should probably do this in some MathML case too, and for SVG foreign object.  And really we should force construction of an anonymous block around inline, text, and br frames.  And maybe also use a different signature for inline Reflow, which is a different protocol.)

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

A simplified testcase would probably help with the crash for this one.

Comment 13

[Jesse Ruderman]

The testcases here are really messy and hard to reduce, so let's work on other bugs for now and see if these crashes go away.

Comment 14

[Jesse Ruderman]

These testcases combine Stir DOM, Random Styles, and creation of random HTML elements and text nodes.  Bug 307989 is the only other bug with such an evil testcase.  I'll say something if I manage to crash while text-zooming or dragging using only Stir DOM or only Random Styles.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 206017 [details] [diff] [review]
patch that fixes tons of reflow assertions (but not the crash) (checked in to trunk, 2005-12-15) (BELONGS ON BUG 320502)

I'm adding an approval request here since there's a chance this may fix some of the other bugs and I don't want to forget about that...

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I filed bug 320502 for further nsHTMLReflowState::mLineLayout issues.

Comment 17

[Daniel Veditz [:dveditz]]

I can't get either testcase to crash in a debug build. The crash I get in an opt build on windows is quite different from the two here. TB13044966, TB13045491

This now crashes at the same windows.dll+offset as bug 310481 but with different JS calls on the stack above that.

xpsp2res.dll + 0x202113 (0x20202113)
js_InitObjectClass  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 1799]
InitFunctionAndObjectClasses  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 1131]
JS_ResolveStandardClass  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 1418]
nsWindowSH::NewResolve  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/dom/src/base/nsDOMClassInfo.cpp, line 5527]
XPC_WN_Helper_NewResolve  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1063]
js_LookupPropertyWithFlags  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 2675]
js_FindConstructor  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 2070]
GetClassPrototype  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 3802]
js_NewObject  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 1952]
JS_NewObject  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 2240]
nsXPConnect::InitClassesWithNewWrappedGlobal  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/nsXPConnect.cpp, line 491]
nsGlobalWindow::SetNewDocument  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp, line 1060]
nsGlobalWindow::SetNewDocument  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/dom/src/base/nsGlobalWindow.cpp, line 809]
DocumentViewerImpl::Init  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/layout/base/nsDocumentViewer.cpp, line 633]
nsDocShell::Embed  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/docshell/base/nsDocShell.cpp, line 4554]
nsDocShell::CreateContentViewer  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/docshell/base/nsDocShell.cpp, line 5660]
nsDSURIContentListener::DoContent  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/docshell/base/nsDSURIContentListener.cpp, line 131]
nsDocumentOpenInfo::TryContentListener  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/uriloader/base/nsURILoader.cpp, line 776]
nsDocumentOpenInfo::DispatchContent  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/uriloader/base/nsURILoader.cpp, line 500]
nsDocumentOpenInfo::OnStartRequest  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/uriloader/base/nsURILoader.cpp, line 345]
nsInputStreamChannel::OnStartRequest  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsInputStreamChannel.cpp, line 359]
nsInputStreamReadyEvent::EventHandler  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/xpcom/io/nsStreamUtils.cpp, line 120]
0x778b0c24
nsFormControlList::QueryInterface  [c:/builds/tinderbox/Fx-Mozilla1.8/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLFormElement.cpp, line 1777]
0xd908245c

Comment 18

[Jesse Ruderman]

0x20202020 is used for deleted JavaScript objects, or something like that, which might be part of the explanation for how the instruction pointer ends up at 0x20202113.  I don't know how you'd end up with an odd-numbered address, though.

Comment 19

[Daniel Veditz [:dveditz]]

Under purify this does look like corrupted memory. I see a handful of invalid pointer writes (IPW) at 

  nsImageWin::CreateImageWithAlphaBits(HDC__ *) [c:\dev\ff15\mozilla\gfx\src\windows\nsimagewin.cpp:434]
  nsImageWin::CreateDDB(void)
  nsImageWin::Draw(nsIRenderingContext&,nsIDrawingSurface*,int,int,int,int,int,int,int,int)
  nsRenderingContextImpl::DrawImage(imgIContainer *,nsRect const&,nsRect const&)
  nsImageBoxFrame::PaintImage(nsIRenderingContext&,nsRect const&,nsFramePaintLayer)
  nsImageBoxFrame::Paint(nsPresContext *,nsIRenderingContext&,nsRect const&,nsFramePaintLayer,UINT)
  nsBoxFrame::PaintChild(nsPresContext *,nsIRenderingContext&,nsRect const&,nsIFrame *,nsFramePaintLayer,UINT)
  nsBoxFrame::PaintChildren(nsPresContext *,nsIRenderingContext&,nsRect const&,nsFramePaintLayer,UINT)
  nsBoxFrame::Paint(nsPresContext *,nsIRenderingContext&,nsRect const&,nsFramePaintLayer,UINT)
  nsBoxFrame::PaintChild(nsPresContext *,nsIRenderingContext&,nsRect const&,nsIFrame *,nsFramePaintLayer,UINT)
  nsBoxFrame::PaintChildren(nsPresContext *,nsIRenderingContext&,nsRect const&,nsFramePaintLayer,UINT)
  nsBoxFrame::Paint(nsPresContext *,nsIRenderingContext&,nsRect const&,nsFramePaintLayer,UINT)

When I break there, even the first one, the various objects look already corrupted. I like Jesse's javascript theory because purify wouldn't know about our internal arena management going wrong. Or since these are native objects maybe it's some other arena (xpc?).

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 206017 [details] [diff] [review]
patch that fixes tons of reflow assertions (but not the crash) (checked in to trunk, 2005-12-15) (BELONGS ON BUG 320502)

I think we actually don't want this on the branch unless it fixes known bugs since it will change Web page behavior.  If we do want it, I should reattach it to its own bug.

Comment 21

[Jesse Ruderman]

The patch from this bug caused bug 321402.

Comment 22

[Jesse Ruderman]

WFM Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060111 Firefox/1.6a1.  I tested with the first testcase about a dozen times.  (Trunk builds from Sept 2005 still crash about 50% of the time with that testcase.)

Comment 23

[Boris Zbarsky [:bzbarsky]]

Would be good to get those crash testcases in somewhere sometime...