292033 - setting document.body.innerHTML hangs/crashes Gecko [@ nsContentList::ContentAppended]

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Veditz [:dveditz]]

James Bercegay from GulfTech Security Research sends us a test case that
lobotomizes Gecko by setting document.body.innerHTML of a complex document to
simple text. On windows Firefox 1.0.3 the chrome still responds (switching tabs,
creating new tabs), but no new content can be loaded. If you start closing tabs
or windows it will crash, usually in nsContentList::ContentAppended() or below.
The one I caught in the debugger was trying to QI what looked like a deleted object.

The basic structure is

<div><div><div><div><script>document.body.innerHTML='BOOM!';</script></div></div></div>
[LARGE AMOUNT OF DATA GOES HERE!]
</div>

where it does appear to require a large amount of data (or perhaps complex
structure -- I couldn't reproduce by replacing the gulftech data with a lot of
simple "Lorem ipsum").

Talkback reports include 
TB5350280K
TB5385467Q

This might be aviary1.0/1.7-branch only, I could not reproduce with a Suite
nightly or a firefox debug build from the trunk.

Comment 1

[Daniel Veditz [:dveditz]]

bz made some trunk changes to nsContentList.cpp that might have fixed this (bug
240851, bug 144072, bug 271709) and bryner did a bit of deCOMtamination
(removing nsIContentList, bug 280746). Or the fix could have been elsewhere.

CC'ing folks to answer the questions:
  Does this look exploitable on the branch?
  Is it as fixed on the trunk as it appears to me?
    (On the branch I don't get the same "boom" as the initial reporter)
  If not exploitable should we close it WFM on the trunk?

Comment 2

[chris hofmann]

stack provided below.  is there any reason to think this is exploitable?  we
usually don't keep crash bugs security sensitive unless it is.   openning the
bug can help to make faster progress on getting the crash fixed and tested.

Incident ID: 5350280
Stack Signature	0x026cd642 9db48a8d
Product ID	Firefox10
Build ID	2005041417
Trigger Time	2005-04-25 11:59:51.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	
URL visited	
User Comments	gulftech crash (security mail)
Since Last Crash	575981 sec
Total Uptime	575981 sec
Trigger Reason	Privileged instruction
Source File, Line No.	N/A
Stack Trace 	
0x026cd642
nsContentList::ContentAppended 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/base/src/nsContentList.cpp,
line 549]
nsDocument::ContentAppended 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/base/src/nsDocument.cpp,
line 1963]
nsHTMLDocument::ContentAppended 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/html/document/src/nsHTMLDocument.cpp,
line 1201]
HTMLContentSink::NotifyAppend 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 4114]
HTMLContentSink::DidBuildModel 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 2350]
CNavDTD::DidBuildModel 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/htmlparser/src/CNavDTD.cpp,
line 707]
nsParser::DidBuildModel 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/htmlparser/src/nsParser.cpp,
line 1315]
nsHTMLDocument::StopDocumentLoad 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/content/html/document/src/nsHTMLDocument.cpp,
line 942]
nsDocShell::Stop 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 3082]
nsDocShell::Destroy 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsDocShell.cpp,
line 3305]
nsWebShell::Destroy 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/docshell/base/nsWebShell.cpp,
line 1259]
nsWebShellWindow::Destroy 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp,
line 1665]
nsWebShellWindow::Close 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp,
line 371]
nsWindow::DispatchEvent 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1067]
nsWindow::DispatchStandardEvent 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1107]
nsWindow::ProcessMessage 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 3754]
nsWindow::WindowProc 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1349]
USER32.dll + 0x8734 (0x77d48734)
USER32.dll + 0x8816 (0x77d48816)
USER32.dll + 0xb4c0 (0x77d4b4c0)
USER32.dll + 0xb50c (0x77d4b50c)
ntdll.dll + 0xeae3 (0x7c90eae3)
USER32.dll + 0xb3f9 (0x77d4b3f9)
uxtheme.dll + 0x3c20 (0x5ad73c20)
uxtheme.dll + 0x1e300 (0x5ad8e300)
uxtheme.dll + 0x1ac7 (0x5ad71ac7)
uxtheme.dll + 0x1b3d (0x5ad71b3d)
USER32.dll + 0xbb15 (0x77d4bb15)
nsWindow::DefaultWindowProc 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1375]
USER32.dll + 0x8734 (0x77d48734)
USER32.dll + 0x8816 (0x77d48816)
USER32.dll + 0xc63f (0x77d4c63f)
USER32.dll + 0xc665 (0x77d4c665)
nsWindow::WindowProc 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1356]
USER32.dll + 0x8734 (0x77d48734)
USER32.dll + 0x8816 (0x77d48816)
USER32.dll + 0xb4c0 (0x77d4b4c0)
USER32.dll + 0xb50c (0x77d4b50c)
ntdll.dll + 0xeae3 (0x7c90eae3)
USER32.dll + 0xb903 (0x77d4b903)
uxtheme.dll + 0x2881f (0x5ad9881f)
uxtheme.dll + 0x1ac7 (0x5ad71ac7)
uxtheme.dll + 0x1b3d (0x5ad71b3d)
USER32.dll + 0xbb15 (0x77d4bb15)
nsWindow::DefaultWindowProc 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1375]
USER32.dll + 0x8734 (0x77d48734)
USER32.dll + 0x8816 (0x77d48816)
USER32.dll + 0xc63f (0x77d4c63f)
USER32.dll + 0xc665 (0x77d4c665)
nsWindow::WindowProc 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsWindow.cpp,
line 1356]
USER32.dll + 0x8734 (0x77d48734)
USER32.dll + 0x8816 (0x77d48816)
USER32.dll + 0x89cd (0x77d489cd)
USER32.dll + 0x8a10 (0x77d48a10)
nsAppShell::Run 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/widget/src/windows/nsAppShell.cpp,
line 159]
nsAppShellService::Run 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/xpfe/appshell/src/nsAppShellService.cpp,
line 495]
main 
[d:/builds/tinderbox/Fx-Aviary1.0.1/WINNT_5.0_Depend/mozilla/browser/app/nsBrowserApp.cpp,
line 58]
kernel32.dll + 0x16d4f (0x7c816d4f)

Comment 3

[chris hofmann]

oops, didn't see that dan was the bug filer.

running with today's win32 firefox trunk build the page starts to load, progress
meter gets to about 75% and then thobber just continues to spin; no crash, and
I'm able to go to other tabs or shutdown the tab that is loading the test url.

Comment 4

[chris hofmann]

I can also load new content in the tab loading the test url, or other tabs as
well.  it does look fixed to me on the trunk.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So on trunk the behavior here changed with the BindToTree landing a few weeks
back... I'm not sure it's completely fixed, but it definitely helps a tad on
that testcase (no asserts, even).

Before that, we start by asserting when a node with a null GetDocument() (to
whit, the div that is child of body here) is passed to
nsContentList::ContentAppended.  Not quite sure exactly what goes wrong after
that, except obviously we end up with the content list holding references to
dead nodes.

We should sort out what happens here even in trunk builds, I think.  We may not
be crashing, but I'm not convinced we're doing anything particularly sane either
(the fact that the throbber doesn't stop is good indication of that).

Comment 6

[Daniel Veditz [:dveditz]]

CC'ing original reporter

Comment 7

[James Bercegay]

http://www.securityfocus.com/archive/1/397913/2005-05-07/2005-05-13/0

Seems there is another XPCOM based crash discovered?

Comment 8

[Boris Zbarsky [:bzbarsky]]

James, there's nothing XPCOM-related about that URL...  And it's not related to
this bug, so it should go in a separate bug report.

Further, the two lines of HTML proposed there just throw a JS exception in FF
1.0.3 and current trunk over here.

Comment 9

[Boris Zbarsky [:bzbarsky]]

The fix for bug 294235 basically ensures that this won't crash on trunk.

I've filed bug 294274 on the memory leak and throbber not stopping issue I saw
on trunk.  The patch there will fix both.

Comment 10

[James Bercegay]

Hi Guys,

I got word back from iDefense today in regards to the exploitability of the issue.

"The crash appears to be the result of a race condition based on the complexity
of the
html page being rendered , and when the javascript is async executed before
the rendering engine is complete. If the javascript is able to delete the html
page objects
before rendering is complete, then the crash will occur as the renderer tries to
access non-
existant objects.

We have tried several techniques but were unable to turn this DoS into an
exploitable
condition. It may indeed be exploitable but the testing time required would be
to great
to explore the chance."

So, it may be exploitable, but very unlikely. I hope this helps.

James

Comment 11

[Boris Zbarsky [:bzbarsky]]

So at this point this is completely fixed on trunk.  If there is no security
issue on branch, I'd rather do nothing on branch -- fixes for this stuff could
easily cause regressions...

Comment 12

[Daniel Veditz [:dveditz]]

(In reply to comment #11)
> I'd rather do nothing on branch -- fixes for this stuff could easily cause
regressions...

OK, so marked the bug fixed and remove the confidential flag since it doesn't
appear to be exploitable?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Sounds reasonable to me.

Comment 14

[James Bercegay]

Hello guys,

I was suprised to not see this bug squashed in the recent firefox 1.0.5 release,
as it seems to have been resolved from what I read here. Would it be safe to
assume that this issue will be completely resolved in firefox 1.0.6?

Kind Regards,

James

Comment 15

[Boris Zbarsky [:bzbarsky]]

James, see comment 11.  There are no plans I'm aware of to attempt to fix this
on the branch.

Comment 16

[James Bercegay]

I see. 

Are there any objections on making this bug public since it does not seem to be
a likely exploitable security issue? I ask this in regards to comment # 12.

I am also wondering if there are any suggested work-arounds for this bug, as
browser crashes can be very bothersome :)

Kind Regards,

James

(In reply to comment #15)
> James, see comment 11.  There are no plans I'm aware of to attempt to fix this
> on the branch.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Work arounds in the script setting innerHTML?  Or on the part of the user?  I'm
not aware of any for the latter case... :(

Comment 18

[James Bercegay]

I was talking about a workaround in regards to the end user :) If there are no
objections I will be disclosing my findings soon, as it seems this is not an
issue likely to be exploited or pose any real security threat. 

Thanks for looking into this issue guys :)

James

(In reply to comment #17)
> Work arounds in the script setting innerHTML?  Or on the part of the user?  I'm
> not aware of any for the latter case... :(

Comment 19

[Daniel Veditz [:dveditz]]

Clearing confidential flag at reporter's request (comment 16)

Comment 20

[Boris Zbarsky [:bzbarsky]]

I think this is WONTFIX as a 1.7 branch bug (that branch is no longer maintained).