311580 - Crash in [@ js_AppendJSString] appending 30,000 <li> elements to an <ol> via E4X.

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Build ID: 2005-10-07-05, Windows XP SeaMonkey trunk.

Summary: Crash in [@ js_AppendJSString] appending 30,000 <li> elements to an
<ol> via E4X.

Steps to Reproduce:

Load the attached testcase.

js_AppendJSString 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsscan.c,
line 911]
XMLToXMLString 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsxml.c, line
2865]
ToXMLString 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsxml.c, line
2923]
xml_toString_helper 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsxml.c, line
6805]
xml_toString 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsxml.c, line
6835]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1163]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1260]
JS_CallFunctionName 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line
4167]
xml_defaultValue 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsxml.c, line
5002]
js_ValueToString 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsstr.c, line
2739]
XPCConvert::JSData2Native 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcconvert.cpp,
line 692]
XPCWrappedNative::CallMethod 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp,
line 1984]
XPC_WN_GetterSetter 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp,
line 1426]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1163]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1260]
js_InternalGetOrSet 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1303]
js_SetProperty 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsobj.c, line
2981]
js_Interpret 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 3324]
js_Invoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1183]
js_InternalInvoke 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsinterp.c,
line 1260]
JS_CallFunctionValue 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line
4183]
nsJSContext::CallEventHandler 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1417]
nsJSEventListener::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/events/nsJSEventListener.cpp,
line 195]
nsEventListenerManager::HandleEventSubType 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1684]
nsEventListenerManager::HandleEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/events/src/nsEventListenerManager.cpp,
line 1785]
nsGlobalWindow::HandleDOMEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/dom/src/base/nsGlobalWindow.cpp,
line 1545]
DocumentViewerImpl::LoadComplete 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/base/nsDocumentViewer.cpp,
line 1022]
nsDocShell::EndPageLoad 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/docshell/base/nsDocShell.cpp,
line 4764]
nsWebShell::EndPageLoad 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/docshell/base/nsWebShell.cpp,
line 667]
nsDocShell::OnStateChange 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/docshell/base/nsDocShell.cpp,
line 4690]
nsDocLoader::FireOnStateChange 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/uriloader/base/nsDocLoader.cpp,
line 1210]
nsDocLoader::doStopDocumentLoad 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/uriloader/base/nsDocLoader.cpp,
line 844]
nsDocLoader::OnStopRequest 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/uriloader/base/nsDocLoader.cpp,
line 665]
nsLoadGroup::RemoveRequest 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/netwerk/base/src/nsLoadGroup.cpp,
line 686]
nsDocument::DoUnblockOnload 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/base/src/nsDocument.cpp,
line 5092]
nsDocument::HandleOnloadBlockerEvent 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/base/src/nsDocument.cpp,
line 5058]
0x778b0c24
0x006e006f

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Testcase; load to crash

Comment 2

[Brendan Eich [:brendan]]

Attachment: forgot to use local root stack in toXMLString code

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 198858 [details] [diff] [review]
forgot to use local root stack in toXMLString code

r=mrbkap

Comment 4

[Bob Clary [:bc] (inactive)]

Checking in regress-311580.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-311580.js,v  <--  regress-311580.js
initial revision: 1.1
done

Comment 5

[Brendan Eich [:brendan]]

Fixed on the trunk.

/be

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using https://bugzilla.mozilla.org/attachment.cgi?id=198852 on
Windows XP SeaMonkey trunk build 2005-10-10-05; no crash.

Comment 7

[Asa Dotzler [:asa]]

Jay, can you poke at Talkback data and see if this is showing up there? We're
beyond the point on taking non-topcrash crash fixes. 

Comment 8

[Brendan Eich [:brendan]]

(In reply to comment #7)
> Jay, can you poke at Talkback data and see if this is showing up there? We're
> beyond the point on taking non-topcrash crash fixes. 

You won't see this in topcrash, yet.  Not enough users are using e4x on the web,
or on the xul platform, to tilt talkback.

Nevertheless, we need this fix, because based on our sorry experience with heap
vulnerabilities, this bug could potentially be exploited.  That risk is higher
than the risk of something in the fix not working, in spite of the VERIFIED
status and the small patch and all other measures of goodness.  This fix uses
existing API to protect local GC allocations.  It's the same fix we have taken
up till now, and are going to take for other bugs on file that are not yet approved.

/be

Comment 9

[Brendan Eich [:brendan]]

Fixed on the branch too.

/be

Comment 10

[Mike Shaver (:shaver emeritus)]

Comment on attachment 198858 [details] [diff] [review]
forgot to use local root stack in toXMLString code

sr=shaver

Comment 11

[Bob Clary [:bc] (inactive)]

no crash firefox 1.5 rc2 winxp/linux

Comment 12

[Bob Clary [:bc] (inactive)]

Note to self e4x/Regress/regress-311580.js: result: CRASHED   type: browser on 1.8.1a3_2006063021 mac(ppc|tel) but I can't reproduce on macppc from 20060702.