70005 - Crash in GC when running XUL with JS disabled

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Mitchell Stoltz (not reading bugmail)]

There is a problem when javascript is disabled but XUL file contains javascript.
Depeneding on the JS in the XUL Mozilla crashes on load or after several reloadings.
The point and type of crash also vary depending on the content.
Sometimes - nojs.html testcase - the problem is the garbage collector and
mozilla jumps
somewhere in ntdll.dll on win2k.
Other times - xul29.xul testcase - mozilla references NULL object and the crash
is immediate access violation.

I don't know whether this is exploitable but it may turn out to be - jumping to
arbitrary addresses
is dangerous.

To test the testcases turn off javascript.

Georgi Guninski

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Attachment: xul31.xul - part of testcase

Comment 2

[Mitchell Stoltz (not reading bugmail)]

Attachment: xul29.xul - part of testcase

Comment 3

[Mitchell Stoltz (not reading bugmail)]

Attachment: nojs.html - last part of testcase

Comment 4

[Mitchell Stoltz (not reading bugmail)]

The bug may not be in the engine, although that is where we're crashing.
Assigning to Phil for triage.

Comment 5

[Phil Schwartau]

OK, here is what happened when I tried to open nojs.html. I haven't had 
a successful WinNT debug build for some days now, so the one I am using
dates back to 2001-02-12. 

WinNT 20010212xx (JavaScript enabled) : no problem
The page loads, and reloads; you see "XXXXXXXXX" loading in the left-hand box.


WinNT 20010212xx (JavaScript disabled) : crash on load! You see the "XXXXXX"
load once or twice, and then you crash. Will attach stack trace below. 
Note: you see this message in the debug console: 


  Document file: ///(local path)/nojs.html loaded successfully

  JS API usage error: the address passed to JS_AddNamedRoot currently holds
  an invalid jsval. This is usually caused by a missing call to JS_RemoveRoot. 
  The root's name is "nsXULPrototypeScript::mScriptObject".


Based on this message, reassigning to Layout component for further triage -
is this the component that owns nsXULPrototypeScript?  

Comment 6

[Phil Schwartau]

Attachment: WinNT stack trace of crash

Comment 7

[Phil Schwartau]

Here's what I found with a debug Linux build: (from 2001-02-19): 
Whether or not JavaScript is enabled, nojs.html refuses to load,
and the browser appears to hang to the user. In the debug console,
you see hundreds of lines like these: 

  Enabling Quirk StyleSheet
  Error loading URL file:///(local path)/nojs.html: 804b0002
  Enabling Quirk StyleSheet
  Error loading URL file:///(local path)/nojs.html: 804b0002
  Enabling Quirk StyleSheet
  Error loading URL file:///(local path)/nojs.html: 804b0002

                etc. 
                etc. 


When JavaScript was enabled, I seemed to be unable to interrupt this
via Ctrl+C. When JavaScript was disabled, I successfully interrupted
the browser and got a stack trace. Will attach that below - 

Comment 8

[Phil Schwartau]

Attachment: Linux stack trace of hang on load

Comment 9

[Phil Schwartau]

Note: all my findings above are for the file nojs.html.


When I tried xul29.xul, I never crashed on it with my WinNT build.
On Linux, once again the page would not even load.


I tried with and without JavaScript enabled on both platforms - 

Comment 10

[Bernd]

This crash should be reassigned, as it crashes in brendans land setting him on
CC (usually I would reassign ....)

Comment 11

[Brendan Eich [:brendan]]

The windows stack trace suggests that a bogus root has been left in the JS GC's
root hashtable.  Reassigning to waterson, although I'm happy to help.  I don't
know what the Linux stack indicates.  Bernd, that could be my bug, but let's
start with the easier diagnosis and cure the windows crash.

/be

Comment 12

[Chris Waterson]

Attachment: only root if something got compiled (diff -wu)

Comment 13

[Chris Waterson]

mstoltz: can you r=? brendan, sr=?

Comment 14

[Mitchell Stoltz (not reading bugmail)]

Looks good, r=mstoltz.

Comment 15

[Chris Waterson]

Attachment: diff -wu, do event handlers, too

Comment 16

[Chris Waterson]

Hmm, after looking at this one more time, I suspect we should probably be 
paranoid with event handlers as well, for paranoia's sake.

Comment 17

[Brendan Eich [:brendan]]

sr=[e-mail]

Comment 18

[Chris Waterson]

Fix checked in.

Comment 19

[Chris Petersen]

Marking verified in the May 22 build.