Last Comment Bug 48031 - Crash Browser with improper removeChild
: Crash Browser with improper removeChild
Status: VERIFIED FIXED
[rtm-][HAVE FIX]
: crash
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla0.9
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
: Andrew Overholt [:overholt]
Mentors:
http://www.mindspring.com/~bobclary/b...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2000-08-08 10:38 PDT by Bob Clary [:bc:]
Modified: 2008-07-31 02:27 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Crash Mozilla when removing Document.documentElement (212 bytes, text/html)
2000-10-14 12:02 PDT, Bob Clary [:bc:]
no flags Details
Crash Mozilla when replacing Document.documentElement (259 bytes, text/html)
2000-10-14 12:03 PDT, Bob Clary [:bc:]
no flags Details
Demonstrate crashing using all combinations of removeChild, replaceChild (2.74 KB, text/html)
2000-10-14 17:08 PDT, Bob Clary [:bc:]
no flags Details
Proposed fix, added missing null-ptr checks. (2.18 KB, patch)
2000-10-15 00:48 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Splinter Review

Description Bob Clary [:bc:] 2000-08-08 10:38:00 PDT
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; m17) Gecko/20000807
BuildID:    2000080712

If you remove the documentElement from a document or remove an Element that has
not been inserted into a document, you will crash Mozilla.

Reproducible: Always
Steps to Reproduce:
Crash 1.   document.removeChild(document.documentElement);

Crash 2.   elm=document.createElement('a');
           document.removeChild(elm);


Actual Results:  Mozilla crashes

Expected Results:  In Crash 1, perhaps throw NO_MODIFICATION_ALLOWED_ERR, but I
am not sure.

In Crash 2, it should have thrown NOT_FOUND_ERR.
Comment 1 Jeffrey Baker 2000-08-08 10:40:31 PDT
Doesn't crash in M18 on Linux.  2000-08-08-08.
Comment 2 Bob Clary [:bc:] 2000-08-08 11:43:35 PDT
Just tried with 2000-08-08-08 on Win2k. It no longer crashes on either test
case.  Crash 1 removed and returned documentElement and Crash 2 throws a
NOT_FOUND_ERR.  Should I just mark this as WORKSFORME or INVALID?
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2000-08-08 15:41:50 PDT
Marking WORKSFORME based on the above comments.
Comment 4 leger 2000-08-16 08:15:50 PDT
Adding crash keyword for fix and/or verification escalation
Comment 5 Jan Carpenter 2000-09-12 12:10:23 PDT
Mass update of qa contact
Comment 6 Bob Clary [:bc:] 2000-10-14 12:01:43 PDT
I was able to crash 2000101320 by removing or replacing documentElement on a
created XML document.

<html>
<head>
<script language="javascript">

var doc = document.implementation.createDocument(null, 'TEST', null);

doc.removeChild(doc.documentElement);

</script>
</head>
<body>
</body>
</html>

nsVoidArray::Count() line 45 + 3 bytes
nsDocument::RemoveChild(nsDocument * const 0x02f0d6ec, nsIDOMNode * 0x00d34090,
nsIDOMNode * * 0x0012e300) line 2906 + 15 bytes
NodeRemoveChild(JSContext * 0x030154d8, JSObject * 0x030818d8, unsigned int 1,
long * 0x02f9f108, long * 0x0012e3b4) line 545 + 25 bytes


<html>
<head>
<script language="javascript">

var doc = document.implementation.createDocument(null, 'TEST', null);
var elm = doc.createElement('NEWTEST');

doc.replaceChild(elm, doc.documentElement);

</script>
</head>
<body>
</body>
</html>


nsVoidArray::Count() line 45 + 3 bytes
nsDocument::ReplaceChild(nsDocument * const 0x02e40f8c, nsIDOMNode * 0x02e39fe8,
nsIDOMNode * 0x02cbc768, nsIDOMNode * * 0x0012e300) line 2831 + 15 bytes
NodeReplaceChild(JSContext * 0x02e16290, JSObject * 0x02d574e8, unsigned int 2,
long * 0x02ecc0c8, long * 0x0012e3b4) line 497 + 34 bytes

Attaching test cases
Comment 7 Bob Clary [:bc:] 2000-10-14 12:02:51 PDT
Created attachment 17135 [details]
Crash Mozilla when removing Document.documentElement
Comment 8 Bob Clary [:bc:] 2000-10-14 12:03:32 PDT
Created attachment 17136 [details]
Crash Mozilla when replacing Document.documentElement
Comment 9 Bob Clary [:bc:] 2000-10-14 17:07:38 PDT
This crash is more general than replacing or removing documentElement.

Document can have Comment, ProcessingInstruction and DocumentType children in
addition to the one Element child (documentElement).

Mozilla 2000101320 crashes when trying to remove or replace any combination of
Comment, ProcessingInstruction or DocumentType.

Attaching testcase where you can choose the means of your crash.
Comment 10 Bob Clary [:bc:] 2000-10-14 17:08:48 PDT
Created attachment 17150 [details]
Demonstrate crashing using all combinations of removeChild, replaceChild
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2000-10-15 00:48:43 PDT
Created attachment 17174 [details] [diff] [review]
Proposed fix, added missing null-ptr checks.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2000-10-15 00:53:12 PDT
Nominating for rtm, this is a crasher (admittedly not a frequent one, but still
a crasher) and it's trivial to fix the problem and there's no risk involved in
fixin this, it's just a matter of doing null-ptr checks before referencing a few
pointers.
Comment 13 David Baron :dbaron: ⌚️UTC-10 2000-10-16 05:44:22 PDT
Oy.  I think this one was my fault.  Sorry.

Patch looks fine (simple null-pointer check).  r=dbaron

Adding rtm kw since I think jst meant to.
Comment 14 Nisheeth Ranjan 2000-10-16 13:52:01 PDT
Marking rtm need info.  This is a low risk fix for a crasher.  If we are going
to respin another rtm candidate build, we should take this.
Comment 15 Nisheeth Ranjan 2000-10-16 13:52:55 PDT
I've looked at the proposed fix.  r=nisheeth.  Now, all we need is a super
review from Vidur.
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2000-10-16 14:59:06 PDT
Vidur says sr=vidur, but form talking to PDT this won't become an rtm++ bug (too
low gain), so I'm marking it rtm- and setting milestone to mozilla0.9
Comment 17 Johnny Stenback (:jst, jst@mozilla.com) 2000-11-08 22:38:49 PST
Patch checked into the trunk, --> FIXED.
Comment 18 Johnny Stenback (:jst, jst@mozilla.com) 2000-11-08 22:40:25 PST
Really marking FIXED
Comment 19 Bob Clary [:bc:] 2000-11-29 08:07:07 PST
verified that attached test cases do not crash Mozilla 2000-11-28-04 on Win2k
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2000-11-29 08:10:38 PST
verified linux build 2000-11-26-21
Comment 21 Prashant Desale 2001-03-06 16:23:16 PST
QA contact Update
Comment 22 Prashant Desale 2001-06-05 12:09:49 PDT
Updating QA contact to Shivakiran Tummala.
Comment 23 Sivakiran Tummala 2001-07-25 10:55:13 PDT
verified

Note You need to log in before you can comment on or make changes to this bug.