371124 - Crash [@ LookupUCProperty] with unminimised testcase, using stirdom scriptfuzzer and xbl bindings

Status: RESOLVED FIXED

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

Description

[Martijn Wargers (dead)]

Attachment: zipped testcase

To reproduce the crash:
- Extract the zipped testcase
- Open file 'parentframe.htm'

Usually the crash happens within 20 seconds or so.

Talkback ID: TB29540796Z
LookupUCProperty  [mozilla/js/src/jsapi.c, line 2642]
JS_HasUCProperty  [mozilla/js/src/jsapi.c, line 3009]
nsHTMLExternalObjSH::GetProperty  [mozilla/dom/src/base/nsdomclassinfo.cpp, line 8850]
XPC_WN_Helper_GetProperty  [mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 962]
js_GetProperty  [mozilla/js/src/jsobj.c, line 3515]
js_ValueToIterator  [mozilla/js/src/jsiter.c, line 395]
js_Interpret  [mozilla/js/src/jsinterp.c, line 2764]

I haven't tried minimising it yet (this could be very tough) or getting a regression range (which also might be difficult, because I could encounter crashes unrelated to this crash).
Marking security sensitive, because it also crashes on branch (with a different stack).

Comment 1

[Jesse Ruderman]

Attachment: reduced testcase

Comment 2

[Jesse Ruderman]

Attachment: reduced testcase

Comment 5

[Martijn Wargers (dead)]

(In reply to comment #3)
> That was hard to reduce.
Heh, yeah, I had a feeling that was hard to reduce, thanks for doing!

Comment 6

[Martijn Wargers (dead)]

With the reduced testcase, the regression range I get is between 2006-05-18/2006-05-23 on trunk and 2006-07-06/2006-07-07 on branch, so I think this  a regression from the js1.7 landing, see:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-07-06+03&maxdate=2006-07-07+09&cvsroot=%2Fcvsroot

Comment 7

[Mats Palmgren (inactive)]

Attachment: Testcase #4

A similar crash occurs from nsHTMLDocumentSH::DocumentAllTagsNewResolve()
after setting "document.all.tags.__proto__ = null"

Comment 8

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Comment 9

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1 (diff -w)

This fixes the crash for both testcases on trunk and branches.
On the 1.8.0 branch we don't reach nsHTMLExternalObjSH::GetProperty()
with attachment 255964 [details] for some reason... the code is the same though
so the crash would likely occur if we do get there with a null proto...

Comment 10

[Boris Zbarsky [:bzbarsky]]

I don't really know this code well enough to do a fast review.  Ask jst?

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 256080 [details] [diff] [review]
Patch rev. 1 (diff -w)

- In nsHTMLDocumentSH::DocumentAllHelperNewResolve():

+    JSObject *proto = ::JS_GetPrototype(cx, obj);
+    if (proto) {

Rather than going past this at all in the case where we have no proto I think I'd prefer to simply bail early here (return JS_TRUE). It's not like we care to expose anything at all in any way if someone sets our document prototype's prototype (i.e. the document.all helper protos proto) to null. If we do what this patch does then an element with name="getElementById" would be reflected as document.getElementById, and that's IMO not desired here.

r+sr=jst with that change.

Comment 12

[Mats Palmgren (inactive)]

Fixed DocumentAllHelperNewResolve() as suggested above.
Checked in to trunk at 2007-02-22 18:18 PST.

-> FIXED

Comment 13

[Daniel Veditz [:dveditz]]

I get a different stack in FF2.0.0.2 and FF1.5.0.10 -- FF2 doesn't look too bad, a null-deref in LookupUCProperty. In 1.5.0.10 I crash on an uninitialized cx->fp (0xCCCCCCCC) in js_AllocStack()

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 256080 [details] [diff] [review]
Patch rev. 1 (diff -w)

approved for 1.8/1.8.0 branches, a=dveditz for drivers

Comment 15

[Mats Palmgren (inactive)]

Checked in to branches at 2007-04-03 07:47 PDT

Comment 16

[Marcia Knous [:marcia]]

While trying to verify this bug, I crashed using the first zipped testcase. My Talkback Incident ID=31937866. I was using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.4pre) Gecko/2007050804 BonEcho/2.0.0.4pre on Windows Vista. Martijn filed Bug 371125 to address this. The other testcases do not crash, so verifiying this fixed on the 1.8 branch and adding verified keyword, per discussion with gavin and martijn on IRC.

Comment 17

[Marcia Knous [:marcia]]

verified on the 1.8.0 branch using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.0.12pre) Gecko/20070508 Firefox/1.5.0.12pre. I did not crash with the testcases listed, including the original zipped testcase. Adding branch verified keyword.

Comment 18

[Bob Clary [:bc] (inactive)]

crash test landed
http://hg.mozilla.org/mozilla-central/rev/b8a601568262