763626 - Crash [@ nsDOMTokenList::ToString] with itemRef, GC

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Attachment: testcase (requires pref) (requires extension for GC)

1. Set
  user_pref("dom.new_bindings", false);
2. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi
3. Load the testcase.

Result: Crash [@ nsDOMTokenList::ToString]

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Boris Zbarsky [:bzbarsky]]

That's really odd.  Why do the bindings matter?

In any case, the key part is that the element went away and didn't null itself out on the domtokenlist.  nsDOMSettableTokenListPropertyDestructor should null out the mElement on the list, I bet.

Comment 3

[Peter Van der Beken [:peterv]]

Ah, yeah, should DropReference().

Comment 4

[David Zbarsky (:dzbarsky)]

I'll write a patch in a little bit.

Comment 5

[David Zbarsky (:dzbarsky)]

Attachment: Patch

Is there a way to convert that testcase to a crashtest?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 632026 [details] [diff] [review]
Patch

r=me, though can list really be null here?

As far as tests.. you should be able to make a mochitest out of it.

Comment 7

[David Zbarsky (:dzbarsky)]

Attachment: Patch with test

You're right, the Element should keep the nsDOMSettableTokenList alive

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 632042 [details] [diff] [review]
Patch with test

r=me

Comment 9

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a2ad5ae1ed47

Comment 10

[David Zbarsky (:dzbarsky)]

Merged to mc by mbrubeck:
https://hg.mozilla.org/mozilla-central/rev/a2ad5ae1ed47

Comment 11

[Daniel Veditz [:dveditz] out until Nov 10]

From the blocking bug 591467 notation I'm assuming you  mean this is a regression from that feature, and I've therefore marked previous releases as "unaffected". If this is incorrect and we need this security fix on those branches please update the status(es) to "affected".

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Not tracking this for advisories since 15 and earlier are unaffected.