If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

"ASSERTION: Wrong scope, this is really bad" with <template>

VERIFIED FIXED in Firefox 32, Firefox OS v2.0

Status

()

Core
DOM
--
critical
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Jesse Ruderman, Assigned: wchen)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla33
assertion, regression, sec-high, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox31 unaffected, firefox32+ verified, firefox33+ verified, firefox-esr24 unaffected, b2g-v1.3 unaffected, b2g-v1.3T unaffected, b2g-v1.4 unaffected, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Created attachment 8437153 [details]
testcase

###!!! ASSERTION: Wrong scope, this is really bad!: 'js::GetGlobalForObjectCrossCompartment(obj) == newScope', file content/base/src/nsDocument.cpp, line 4458
(Reporter)

Comment 1

3 years ago
Created attachment 8437154 [details]
stack
Keywords: sec-high
mTemplateContentsOwner::mWillReparent is not set to true when calling SetScriptGlobalObject here: http://hg.mozilla.org/mozilla-central/annotate/18c21532848a/content/base/src/nsDocument.cpp#l4507. Maybe it should be set to the same value as the caller's mWillReparent?
(Assignee)

Comment 3

3 years ago
Created attachment 8439550 [details] [diff] [review]
Reparent template contents owner document.

Following your advice I've set mTemplateContentsOwner::mWillReparent to true and added some code to reparent the contents owner document.

Some alternatives could be to not change the global of mTemplateContentsOwner except for setting it to nullptr. We could also just set the global to nullptr when the document is created and keep it that way. I'm not sure too sure about the consequences of these options given that the template contents owner is a data document.
Assignee: nobody → wchen
Attachment #8439550 - Flags: review?(peterv)
status-firefox33: --- → affected
Comment on attachment 8439550 [details] [diff] [review]
Reparent template contents owner document.

Review of attachment 8439550 [details] [diff] [review]:
-----------------------------------------------------------------

We'll need an automated testcase. Can the template content's document be accessed by script? It needs a global if so. And we need a testcase that does that after document.open() too. r+ assuming the document is exposed to script.
Attachment #8439550 - Flags: review?(peterv) → review+
Component: HTML: Parser → DOM
(Assignee)

Comment 5

3 years ago
Created attachment 8445580 [details] [diff] [review]
Mochitest testcase

Testcase as requested.
Attachment #8445580 - Flags: review?(peterv)
(Assignee)

Comment 6

3 years ago
Comment on attachment 8439550 [details] [diff] [review]
Reparent template contents owner document.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Not easily.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
The comment suggests what the problem is.

Which older supported branches are affected by this flaw?
aurora

If not all supported branches, which bug introduced the flaw?
bug 1017896

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Patches should apply cleanly.

How likely is this patch to cause regressions; how much testing does it need?
Not likely to cause a regression, opening the attached testcase in debug build without assertion and passing the attached mochitest testcase should be sufficient.
Attachment #8439550 - Flags: sec-approval?
Comment on attachment 8439550 [details] [diff] [review]
Reparent template contents owner document.

sec-approval+ for trunk. Once it is on trunk and trunk is green, please nominate an Aurora patch so we can avoid shipping this issue.
Attachment #8439550 - Flags: sec-approval? → sec-approval+
status-firefox31: --- → unaffected
status-firefox32: --- → affected
status-firefox-esr24: --- → unaffected
tracking-firefox32: --- → +
tracking-firefox33: --- → +
(Assignee)

Comment 8

3 years ago
Created attachment 8446232 [details] [diff] [review]
Mochitest testcase
Attachment #8445580 - Attachment is obsolete: true
Attachment #8445580 - Flags: review?(peterv)
(Assignee)

Comment 9

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a5e4f7ea2cb
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/0a5e4f7ea2cb
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox33: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
(Assignee)

Comment 11

3 years ago
Comment on attachment 8439550 [details] [diff] [review]
Reparent template contents owner document.

Approval Request Comment
[Feature/regressing bug #]: Bug 1017896
[User impact if declined]: Malicious scripts might be able to access objects in an old document global.
[Describe test coverage new/current, TBPL]: Code has a debug assertion that is executed by the included mochitest.
[Risks and why]: No risk to taking this patch.
[String/UUID change made/needed]: None
Attachment #8439550 - Flags: approval-mozilla-aurora?
Attachment #8439550 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/ff12bd072d73
status-b2g-v1.3: --- → unaffected
status-b2g-v1.3T: --- → unaffected
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
status-firefox32: affected → fixed
Confirmed assert on Fx32, 2014-06-10
Verified fixed on Fx32, Fx33, 2014-07-14.
Status: RESOLVED → VERIFIED
status-firefox32: fixed → verified
status-firefox33: fixed → verified
Blocks: 1017896
Group: core-security
Keywords: regression
You need to log in before you can comment on or make changes to this bug.