"Assertion failure: win->IsClosedOrClosing()" with XSLT, carefully-timed GC

ASSIGNED
Assigned to

Status

()

--
critical
ASSIGNED
6 years ago
6 years ago

People

(Reporter: jruderman, Assigned: johns)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
assertion, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 631243 [details]
e.html

1. Save e.html
2. Save 458637-1-inner.xhtml
3. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi
4. Load e.html

Assertion failure: win->IsClosedOrClosing(), at dom/base/nsDOMClassInfo.cpp:2012

This assertion was added in bug 691178.
(Reporter)

Comment 1

6 years ago
Created attachment 631244 [details]
458637-1-inner.xhtml
(Reporter)

Comment 2

6 years ago
Created attachment 631245 [details]
stack trace
Assigning to bholley to answer how bad this assertion is, and figure out if this is a problem in the DOM goop or in XSLT itself (and reassign appropriately)
Assignee: nobody → bobbyholley+bmo
(In reply to Jesse Ruderman from comment #0)
> This assertion was added in bug 691178.

It was actually added in bug 640904.
I'm pretty swamped with security bugs (did 6 last week).

jst and I were talking about this in our 1-on-1 today, and jst thought this might be a good thing for johns to look into. John, can you have a look? It's more or less going to involve crawling around and figuring out exactly how the testcase manages to call into the nsDOMClassInfo PreCreate code when the window has no JS object.

Feel free to ping me for any input I can provide. If this is your first time poking around nsDOMClassInfo stuff, this conversation might be useful: http://people.mozilla.org/~bjacob/bholley-on-wrappers-scriptablehelpers-expandos
Assignee: bobbyholley+bmo → jschoenick
Hi John, any traction on this one?
(Assignee)

Comment 7

6 years ago
So the culprit is here:
http://hg.mozilla.org/mozilla-central/file/f2146a6c104e/content/xml/document/src/nsXMLContentSink.cpp#l372

When XSLT fails, it generates a simple error document, and returns it to nsXMLContentSink. It then gets swapped into the docshell, and EndLoad() called on the now-replaced document. This means the DOMContentLoaded event is queued and fired for the old document, which is now sharing its JS Object with the window of the new document. However, SetDOMDocument un-roots said JS object, so the GC can race with it and destroy it.

What we should be doing is calling SetScriptGlobalObject(nullptr) on the old document as it should not be associated with that window any longer -- but because we're still loading, this will cut off the final onload event from firing, meaning onload for an XML/XSLT iframe might not fire in case of failure.

I'm looking into a proper fix
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86_64 → All
(Reporter)

Comment 8

6 years ago
The assertion in comment 0 was removed in bug 784792.  But it sounds like the underlying problem in the XSLT case is not yet fixed.
(Reporter)

Updated

6 years ago
Blocks: 742139
Keywords: sec-moderate
This isn't a security bug (also, the assertion we're hitting here was backed out because it was deemed to be incorrect). The failure here is conservative, since we will refuse to hand out any new wrappers for the given scope meaning that we should be safe.
Group: core-security
Keywords: sec-moderate
You need to log in before you can comment on or make changes to this bug.